Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <286146b0-51bb-a1d0-fa28-50e5a792aeed@oracle.com>
Date:   Mon, 24 Oct 2022 16:05:30 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH] fs: jfs: fix shift-out-of-bounds in dbDiscardAG
Content-Language: en-US
To:     Hoi Pok Wu <wuhoipok@gmail.com>
Cc:     jfs-discussion@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        syzbot+f0e0fcf3cd1047ae60ad@syzkaller.appspotmail.com,
        syzkaller-bugs@googlegroups.com
References: <20221019093825.339991-1-wuhoipok@gmail.com>
From:   Dave Kleikamp <dave.kleikamp@oracle.com>
In-Reply-To: <20221019093825.339991-1-wuhoipok@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aGY2Y0Y1Q08xLzE1WEZGWjJWc2I0NHNzczFRQ24zSWVwT3JHWTl3Q1RSZmxE?=
 =?utf-8?B?aDdNVyt5Z0dHejlrUDVJZkRMUHlCWXRsNUVSL09WK3hoVUpBS1VmemRCcWVT?=
 =?utf-8?B?MGpEZVNySWl0K0JaWlYzSzdjTjlmK0tTWHdYR1QzS2RGQW9nVTNkTW95ZUtw?=
 =?utf-8?B?bEIwcUc2WWFiMDZKVytzYXFGVUtzdFkvOStXMzBQbHNoWXFNT0dyU0doeHo4?=
 =?utf-8?B?NTJtMllQeU5FVm91ZFUwdEZENUY3SzN4QU5TeHZ6TDlKaWlObmViamwyTlA3?=
 =?utf-8?B?NUNMTkM4WG5yYnNpMjNaeEsxMi9JZlluY0l3V09xNTFvV0UxdWU2VHVMOTF0?=
 =?utf-8?B?QVBsWUFyK1c0QXNGNCtZMEdCWEZKaXFHeFRBWWN0djhJN21NSVcweUhwK05D?=
 =?utf-8?B?TS9PUDFMOUptSTNsVlY0aitEL2ZkYnd6UXVKLytrTlJFWmtnNkFDKytobERs?=
 =?utf-8?B?dVRrckxLMnl5dFFScnJJVDJPWHYrL0o5cFJxTDRTbHQrdkNHQVI3dnE5L1A1?=
 =?utf-8?B?VG5TamtXZlh5Q0RqeVB5dDNETE1DVS9nWXBaeFZpUGgzdHkzaHQxYTNTMzdO?=
 =?utf-8?B?ckxwditnbVRwWDBFbVR0eDVFc05wV0V3RWFqeGFtMFJlclFxaExQYU0vQ0lw?=
 =?utf-8?B?NDhtbUlLVXBQK1BWSEN1T0xQT21RNUtWYjV3Q1dHdHJBRHM2ZmNZdzZHQjBO?=
 =?utf-8?B?TmZYUTNzMklqUmxmUEZSdWhLYnRUTFNUemFrK0lYSDd5NHBvZ3ZoTmlYbWtE?=
 =?utf-8?B?dDdhNGRoQ1o2TXJxc0wyRm1NTG8wK0FLS3hWLzRNd05nTmVxeXR3eGU2TStv?=
 =?utf-8?B?Z3krSUZxK1htT1pIRUNmak5zN2hEanR0VzNMOFNOWlFua2dpQWZNK01rSHk2?=
 =?utf-8?B?dm5UU2hacUMvK3ZmeENld3htME1WTEd1Y3MyQkpXVE84RmxPbmVTN1plQkcv?=
 =?utf-8?B?UUszUm83OGFsa1hNdDcrUXVoQXdRRGpqWTg0L2h1am1udlE5NTl2dVBEWkFx?=
 =?utf-8?B?OTZwYzh4TEt5aU01cUc0eEo4RENSQ2gzMVo0RU1xT0s4ZjZ1Sm9RZjVhVHVj?=
 =?utf-8?B?QS9wSGx4U3QycmJPdVZWKzlQS290eXpNUTFQV01lM3hvR3NyNTRYYW5teDRD?=
 =?utf-8?B?bk4zVGRaM1RTSVUrdlRqb2JFSlhvSFczRE9kY3h6QS9ZWmpYdnJZcjc3TWxn?=
 =?utf-8?B?enNxakcrWStnR3hLZ1VTc0ZGOVJkZ2pjeFlkTHpPd1NmdkEvc1JRUHMyekNC?=
 =?utf-8?B?Z3NjRkM0NGRWRnZpdUp3dnhKaTVONnFubHRuWXQ2NmV3S3QyZHQyamdtajRB?=
 =?utf-8?B?YzFndjJORFdMNVF5WHErV0lHY3hRZTMwU3RxS1hSNFBpOXJSaXEwRDNUb2FY?=
 =?utf-8?B?S2pOYnJsTVdDaFE4ODNiakEvWjJXY3RKNnFyM3JUeHpEbng5NGFtRWFISmcv?=
 =?utf-8?B?b2JvOU5zSFNFczd5dHBURHZMenJwdHZGVWxaMXBRWTRGWCtkUEluN0phSmxL?=
 =?utf-8?B?czZzZmwreG4vaHZyZE9UWVNnczA4MFF3R3NuQ0RObngwVU1zQjN3UkU5RS9Q?=
 =?utf-8?B?SG0yZC9NZGJOQklWcWNiM2srWk54RFdRSVltMERNYXkrWktFQXhOYjR6ZGMv?=
 =?utf-8?B?RVFZd2t5dlliQzJPTG0wOTlNSVlxR2RsVXowK1c2TkpZUk5xTVVickVXYUFa?=
 =?utf-8?B?ZW9LS3RNVG8zTzZpQ0Vla1FrbzdFK2Q0a3J5V2VFZmtvUXRnWUVhNFhMck02?=
 =?utf-8?B?YjJqZkJ2bnR1cnlBNzh0OGg0RytQbDUxV3llU212cHJ0RGxuYXpiSUtkWllM?=
 =?utf-8?B?alNsUjUrdFc5L1B3NEJ4bjZScFZXd3RVUk42RlY4dURMblh6MFVHTlFPTjhs?=
 =?utf-8?B?bGgzRFRtOWpDQ1Yrcld0UDl6VUJUeVRVSzhqK3hPV1IyOFdBdzVnVkhoVFl3?=
 =?utf-8?B?VUp1akVQWGJSa1RxbU5CdjlYWUFiTE96N2ZiTXZIUFVCcGZIRzdvQWJhNHIv?=
 =?utf-8?B?SEQrOG5rRlFxOHZycXVqUDdESy9weERvaTMzQUtocmpDNWJrUXpaTWI0WTRp?=
 =?utf-8?B?Y1ZrVEdZYjNadnN5Z3ExT2l0alpaNCt0TGR0aXYrbUgrMjRha0JrdkZzeGJ5?=
 =?utf-8?B?bGdreXNMNUJpbVRQUXhEZktEUmNSbTV1N1NPL1BleWFZc0gzaHFiaEtYY3NS?=
 =?utf-8?B?dWc9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 98c86430-8014-4f53-dc9b-08dab6037d37
X-MS-Exchange-CrossTenant-AuthSource: MW5PR10MB5738.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2022 21:05:33.0503
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: hE3PUQsuPxwLQkCod/SETnSGIz+Xx1L2tQKb0IwzQQJiBRoKSBKFRtvvsw28jg1RPEort2dZ9edzqlxMaNS7gQWNx3zXuJWFTAPoCHJq+EE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5648
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-24_07,2022-10-21_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxscore=0 phishscore=0
 mlxlogscore=999 adultscore=0 malwarescore=0 spamscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000
 definitions=main-2210240126
X-Proofpoint-GUID: wqnbjppyQ6qLSR_fMZhTq8pGUsKpgQSt
X-Proofpoint-ORIG-GUID: wqnbjppyQ6qLSR_fMZhTq8pGUsKpgQSt
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/19/22 4:38AM, Hoi Pok Wu wrote:
> Bug reported by syzbot, UBSAN: array-index-out-of-bounds in dbDiscardAG.
> Fixed by simply rearranging agno_end smaller than MAXAG. However, I wonder if it
> is a better idea to return -EINVAL, rejecting user input. Thank you.

I don't think the problem is simply that the user input is bad, the 
range is already limited:

         if (end >= bmp->db_mapsize)
                 end = bmp->db_mapsize - 1;

It seems syzbot corrupted some on-disk data structure to trigger this 
failure. It may be best to call jfs_error() and bail out.

Thanks,
Shaggy

> 
> Reported-by: syzbot+f0e0fcf3cd1047ae60ad@syzkaller.appspotmail.com
> Signed-off-by: Hoi Pok Wu <wuhoipok@gmail.com>
> ---
>   fs/jfs/jfs_discard.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/fs/jfs/jfs_discard.c b/fs/jfs/jfs_discard.c
> index 575cb2ba74fc..3101eaf3098a 100644
> --- a/fs/jfs/jfs_discard.c
> +++ b/fs/jfs/jfs_discard.c
> @@ -96,6 +96,10 @@ int jfs_ioc_trim(struct inode *ip, struct fstrim_range *range)
>   	 */
>   	agno = BLKTOAG(start, JFS_SBI(ip->i_sb));
>   	agno_end = BLKTOAG(end, JFS_SBI(ip->i_sb));
> +
> +	if (agno_end >= MAXAG)
> +		agno_end = MAXAG - 1;
> +
>   	while (agno <= agno_end) {
>   		trimmed += dbDiscardAG(ip, agno, minlen);
>   		agno++;