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: <9e4e1fd8-3ff7-dcce-428f-88dbd5fddd09@oracle.com>
Date:   Mon, 17 Oct 2022 15:02:02 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Subject: Re: [PATCH v2] fs: jfs: fix shift-out-of-bounds in dbAllocAG
Content-Language: en-US
To:     Dongliang Mu <dzm91@hust.edu.cn>
Cc:     Dongliang Mu <mudongliangabcd@gmail.com>,
        syzbot+15342c1aa6a00fb7a438@syzkaller.appspotmail.com,
        jfs-discussion@lists.sourceforge.net, linux-kernel@vger.kernel.org
References: <20221016012507.428006-1-dzm91@hust.edu.cn>
From:   Dave Kleikamp <dave.kleikamp@oracle.com>
In-Reply-To: <20221016012507.428006-1-dzm91@hust.edu.cn>
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?VkVTZHdEelUwS3c3TTRqOG1FdjRQTUgyZVF5RmZSVUJQUGxCUTQzQmFFYlJ2?=
 =?utf-8?B?ZnhRdENDSVFKOXZOb2YvU3MvYXpNT21IRno5L2g4SmJvd050YlVTdmlVL2Nh?=
 =?utf-8?B?SnNJMllYSS8rM1BONzVFcmVuakdpN2pZeTRnVmpFMXFXQkt3N2htc2h4V1hp?=
 =?utf-8?B?empWMncyekJJQ0dUWkk0WXlSUC9JMnBVNHRUaVhYQnN4VUdOdkZ4NzZhUmlO?=
 =?utf-8?B?cW4vQmduYVlqMi9FM3FWUm8rL3NXUzU5VU8xejdKZGY3YUJnaXQySDA0WE5x?=
 =?utf-8?B?Y3JVckFsVzR6SWZqZzAxc1dHY24zN2tyaFMzTXN2S2VySWxmZWEvTHZtdkM5?=
 =?utf-8?B?K3JrbGxqaEZKYjl1OE9GQUFRcU94RkNGVlJKZmlRNWluM3E5cnBZUXNZd1lB?=
 =?utf-8?B?UTJBR0F2cmlVUGNGTVZmbm85Zk9ZZS92SUM4UkpPdnNqZkFnSUVzUWJseXN0?=
 =?utf-8?B?TktXdTBHcG9PR1dIYW5Ub0RndEJLaVZNREFPdG9FL1NXQVBCZ2tveWFvMjVJ?=
 =?utf-8?B?WVhCRXVyY3RpSmIxODYwZHZLanNmZTloUytjQkhHaFZuOXNKUXVXYXhKc0JP?=
 =?utf-8?B?RmRhWXcydkt6Ujdqd1FlWDRCWTYxamRVRW94ak1OdHJNejNZZndWZFJsOW1E?=
 =?utf-8?B?aTBRSTVYUHNQa0l3eXY1NlhqL0lKYjhqVnlzZkE5dEZYdy9GMVd5ekZ0Q0RG?=
 =?utf-8?B?VVVpUVl4WU54cEF4L0piVW4rY205dm1RVE5iUDFWamFHTExKc1NZMVU1QjRy?=
 =?utf-8?B?aDNDcjVPblJLZHBhSG9WRWh1TVE3bXR0Q0hmQkVuakl6WVJnMHRsK3NoQ2lU?=
 =?utf-8?B?dUxiY2RvYkZobStGdUc4RmgxY3RUc28zcGVzRDBuUEZyMlVKeCtmRUF0WXN1?=
 =?utf-8?B?U0lmM2lZb28vTGtPYk1JRnNQaUlhcXNZaVRkNHZYaUFGVVNaWmxLM1ZXbFU2?=
 =?utf-8?B?NjY3MStPRUlUUEM5MGVKeXZXSWFsY3VpMWZVQnNLWkZqZGdQMXdRNStoV1lY?=
 =?utf-8?B?ZGkzRzVpQklTaitFcGtuVjBlRWRCWm4yREtSSEhjV29OMm5PMkxRazJGeHlR?=
 =?utf-8?B?OG80MVJVSkpOWUwxc2hkUG5US3JZK1BlUFc1bDJkdzVtRjZvL3prTUlVR05q?=
 =?utf-8?B?aElDQTNnTzhLL2h6N3hQakFLOUNPbHkxWWpyVDFQSGZqamtzZjRQTU5lTFAv?=
 =?utf-8?B?MlMzR05XR2xaSTNSOWxjbXF0cTBDRzk4eGd0amRRbSswck9ic2cwdGV5UjRE?=
 =?utf-8?B?eHNGNk0yejdEbFVrNG42OVcwSEovcytjM2N6TkVSRFVVejNiQ2Fvelg5M0xM?=
 =?utf-8?B?RTl5eHh0VWE1UVJHWlNyZ3RPNnNHUFo4TmtCS3JCVTJBMlVoUVJ2RmlCNmZ6?=
 =?utf-8?B?ZXhlRkJHYloxMEJydlQ5K09LZkdLeHI3YlBZNGcrN000WGwzNDNIVndEdHFt?=
 =?utf-8?B?b0MxWUNhUnJLVjkvcUUxbnZUcmZnSE9ZdFhqeXRONDlTaWMvd3JvblpKdXlS?=
 =?utf-8?B?engwbmhRNkhsTjNoQmU4L0NVS2s5ellXUzdaSWFJMjdSV29acTNoOTV6dTgv?=
 =?utf-8?B?eG42TG9EUUdMMThCNkIyb0xZODhsUmVsaGNIVm82bkE0azRBT3NJdFIyTk1s?=
 =?utf-8?B?N2ZlcXU0R29nMHluaGd1K1lhM1ozdnBtZzdBQXhBaGRDZlp5RXVHWkRlMmpQ?=
 =?utf-8?B?aC9SWk4wMHJkYmFOUVBmSFNjdVVmNXFKaVV1NmdSRGRVSUtuWE5WMUQzVzZu?=
 =?utf-8?B?NTVLeENjQ080NytpZ1g1eld1RnQyTm82Umt3QXlIVmYzYWVyTk01S2d5R25s?=
 =?utf-8?B?bXMyblhrdU9hUU5sMDFqZDNRTUMyQVJ5Y05OZ0pUOGRGQy9Icm13cGR4V0xs?=
 =?utf-8?B?RSt6WTV6NGxuYVBmUUFHMm9HcTRzWU9ST3VVSU1kNE5FeGN1amxvS0lUaHpz?=
 =?utf-8?B?MmNlbE1DY1pTNndCREJKK3YzaE51WjFJR2lnc085R3d0SE9XVlJ6SWQvYWgz?=
 =?utf-8?B?OUVlZ0JCWE94SDdnTjlGWk40a1VRWThDNCt6SVBFMjVuaVdZVjI1UE8vZUp5?=
 =?utf-8?B?cGZnL0VQcWl4WmJ4T2tTbTd5ZEhCV1Q1N29DNDNCaXAveFQ3U2RpbjUzMlVF?=
 =?utf-8?B?bUJ0UHNCQ21PSnRuR0hsNjBaeHlDVXVDZ3ViRGhFelY1YVJPT2p2Nk5sbDJG?=
 =?utf-8?B?SVE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 28144617-eeec-4f14-6f2b-08dab07a7686
X-MS-Exchange-CrossTenant-AuthSource: MW5PR10MB5738.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Oct 2022 20:02:05.0022
 (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: AdqXk4TlIvso6CXCdcoRiUOtkMU+J9vpwifi3eXXoYXLvR7eInuHrXbgstSZL0tj14gToySB+3i5//RKm+Z24odXimEladz1lsCra7qYQ1g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5314
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-17_13,2022-10-17_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 phishscore=0
 suspectscore=0 spamscore=0 adultscore=0 mlxscore=0 malwarescore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2209130000 definitions=main-2210170115
X-Proofpoint-GUID: TKc3aI9IDTl1PtUeXFwUa9OzvmeSbHhA
X-Proofpoint-ORIG-GUID: TKc3aI9IDTl1PtUeXFwUa9OzvmeSbHhA
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

Looks good. Pushed to jfs-next.

Thanks,
Shaggy

On 10/15/22 8:25PM, Dongliang Mu wrote:
> From: Dongliang Mu <mudongliangabcd@gmail.com>
> 
> Syzbot found a crash : UBSAN: shift-out-of-bounds in dbAllocAG. The
> underlying bug is the missing check of bmp->db_agl2size. The field can
> be greater than 64 and trigger the shift-out-of-bounds.
> 
> Fix this bug by adding a check of bmp->db_agl2size in dbMount since this
> field is used in many following functions. The upper bound for this
> field is MAXMAPSIZE - L2MAXAG, thanks for the help of Dave Kleikamp.
> Notethat, for maintainance, I reorganzie error handling code of dbMount.
> 
> Reported-by: syzbot+15342c1aa6a00fb7a438@syzkaller.appspotmail.com
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->2: fix the size of bmp->db_agl2size; revise the correct upper bound
>   fs/jfs/jfs_dmap.c | 22 ++++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index 6b838d3ae7c2..962c855631cb 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -155,7 +155,7 @@ int dbMount(struct inode *ipbmap)
>   	struct bmap *bmp;
>   	struct dbmap_disk *dbmp_le;
>   	struct metapage *mp;
> -	int i;
> +	int i, err;
>   
>   	/*
>   	 * allocate/initialize the in-memory bmap descriptor
> @@ -170,8 +170,8 @@ int dbMount(struct inode *ipbmap)
>   			   BMAPBLKNO << JFS_SBI(ipbmap->i_sb)->l2nbperpage,
>   			   PSIZE, 0);
>   	if (mp == NULL) {
> -		kfree(bmp);
> -		return -EIO;
> +		err = -EIO;
> +		goto err_kfree_bmp;
>   	}
>   
>   	/* copy the on-disk bmap descriptor to its in-memory version. */
> @@ -181,9 +181,8 @@ int dbMount(struct inode *ipbmap)
>   	bmp->db_l2nbperpage = le32_to_cpu(dbmp_le->dn_l2nbperpage);
>   	bmp->db_numag = le32_to_cpu(dbmp_le->dn_numag);
>   	if (!bmp->db_numag) {
> -		release_metapage(mp);
> -		kfree(bmp);
> -		return -EINVAL;
> +		err = -EINVAL;
> +		goto err_release_metapage;
>   	}
>   
>   	bmp->db_maxlevel = le32_to_cpu(dbmp_le->dn_maxlevel);
> @@ -194,6 +193,11 @@ int dbMount(struct inode *ipbmap)
>   	bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth);
>   	bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart);
>   	bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
> +	if (bmp->db_agl2size > MAXMAPSIZE - L2MAXAG) {
> +		err = -EINVAL;
> +		goto err_release_metapage;
> +	}
> +
>   	for (i = 0; i < MAXAG; i++)
>   		bmp->db_agfree[i] = le64_to_cpu(dbmp_le->dn_agfree[i]);
>   	bmp->db_agsize = le64_to_cpu(dbmp_le->dn_agsize);
> @@ -214,6 +218,12 @@ int dbMount(struct inode *ipbmap)
>   	BMAP_LOCK_INIT(bmp);
>   
>   	return (0);
> +
> +err_release_metapage:
> +	release_metapage(mp);
> +err_kfree_bmp:
> +	kfree(bmp);
> +	return err;
>   }
>   
>