Received-SPF: pass (google.com: domain of linux-kernel+bounces-158707-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <674f1ee0-4b7a-41c2-8bc5-82bb0f490565@oracle.com>
Date: Thu, 25 Apr 2024 09:24:43 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree
To: Matthew Wilcox <willy@infradead.org>, Jeongjun Park <aha310510@gmail.com>
Cc: brauner@kernel.org, jfs-discussion@lists.sourceforge.net,
        jlayton@kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, shaggy@kernel.org,
        syzbot+241c815bda521982cb49@syzkaller.appspotmail.com,
        syzkaller-bugs@googlegroups.com
References: <ZipSO4ITxuy2faKx@casper.infradead.org>
 <20240425141038.47054-1-aha310510@gmail.com>
 <Zipl4PQ9Q7sBlMCt@casper.infradead.org>
Content-Language: en-US
From: Dave Kleikamp <dave.kleikamp@oracle.com>
In-Reply-To: <Zipl4PQ9Q7sBlMCt@casper.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?RlhsRXlnK2pGcVhybWpNekFXRDVFVDBxWTVoR0dQUk1NTEF3VlRIVXdlSDVS?=
 =?utf-8?B?MDlBS0wyTmt5allhWkVWbjRMOUc0enk2bWI4WDh6T3dReHU4a1laVTVGdnNI?=
 =?utf-8?B?NzFjcm5tdEtXMCtnSlNUeWxQWkNBMCtTWUFSRlZlNXJXQ3FaT2JSUEpBTFM4?=
 =?utf-8?B?THFOT0RmeE1VWFp3VWJtSVh5WUFPZjk2WWRDWmp2OFR0b3BsZ09iWDJjYVVi?=
 =?utf-8?B?WkhQeEVyOWRXSmpiMWtBTzdZQjBMOEdYcVpkNk1kOGFEZmVuT2gyd0NvK0lt?=
 =?utf-8?B?aFdBSkN2WFZZa2lWU0p0WXNsc3k3dlZVdzl6Q3dhWTgvMHVFcDhtRmd1RVN2?=
 =?utf-8?B?SWJHODF5VlZxb0E1U0d3L1d3SHJnblg0UktyQlZ6dGlHL1Y5MzlzWUo2WG1k?=
 =?utf-8?B?UUJyUlZmc0RMTzgxNTRpQ1lyZTVyTFBCQ1ZBb1lWY2VRMmhFMWRlREhOQkEv?=
 =?utf-8?B?NzNOZGlRR3ZHQlVYZE90V0k4UDB6QmhLL2h5eVp2OXNOSGo4ZWhQYlpxbWVo?=
 =?utf-8?B?R1JVQm9ZdFpDM3hud25sTU1ua2NybG1NbkRnOThLQTZrK1Fxeko2SnFwYlE1?=
 =?utf-8?B?SVJFMDRmTzBiZmxqVHpsQVEyL093SE1IMzVKNC9UUmlNVEU1QXQ1UGJRYWhB?=
 =?utf-8?B?aFZYMVl1VDQ2THFOZmV0SVZ5bHNTeWhEY1Y5OWFGeWhweUxCL0VTUDdYNC92?=
 =?utf-8?B?SlVxUk9EUW5tY09TMlpLbER5R0lCRkpReWN2MFROMlhYbm9VTTJVSlp1dVJ4?=
 =?utf-8?B?Vm5pRlJGTFNISUhGS1JHQ1VMT1cwUU55OG9MZW1NaXovK2s4MXRKMVZ6cTlY?=
 =?utf-8?B?WERHZ2F5eWNNQVV2UWhxVVR2UUtTUmZhaGZ4cFU0NkFrRkk1Y3ZXcnlZTXE0?=
 =?utf-8?B?UW00R1pWeDIrVXREMzlDQlFaOUdZc0toYjhBczRrLzhmSWVVdG1BNFBUM3d5?=
 =?utf-8?B?THZIalQvZDZIZG12T0xnMGVvbEU4dHNJKy92R0NGV0FKTHFya2pzZUQ0bDNI?=
 =?utf-8?B?R3F4VFZwb3F3OS9IckdmdkhLTU5TMExjdkNvcStZNXZQMllRQ2Z5cmFHOEtl?=
 =?utf-8?B?OHVPb1JzQW1NR0d3Vnc1TFZBcGNUOGhQdCtiWnNHTCtBaVB2TE05dVAxS3VU?=
 =?utf-8?B?dlh6a1lWa1Y0eWRJOWZwWkJGZ1BpVzlRV0pJZWhxVUVTOTJnS2wwanVEYkRO?=
 =?utf-8?B?SU9QUFV2dUYyWGRzSkNReGxmS3RvSVZTcUpTdTBMS3FhMktHR2ZidlJrb2ph?=
 =?utf-8?B?dUpNZlpvcDdVREZReHlUQVp2cTd0Y1NhU01tUE9rYXBwSmhsNkY0YXR2Qk1Q?=
 =?utf-8?B?WjZSY3k3UFRxRFY2Z3loV3cvcG8xOGJ4NHJLbFZHbnE3T2xrRTl0SmlKTE1X?=
 =?utf-8?B?YVF6OHordmVyczFNV0I5N1NaVHFUM1pvNWdQazVlb1owdmxUcXlMdFpTQnBW?=
 =?utf-8?B?NlF3S01WVFlFanNSbi9HZzljU21GL1FGZGYyMVlHL0U2YktscUxza3JTSTVa?=
 =?utf-8?B?Zmp2ZUgzbEt3WkpaZnZoU2xrUVh2bWR4ME5Fc1VaM2lpL1c5SFBheTlLRE9a?=
 =?utf-8?B?RnVLMmkzalBCeHVLTHVXWWxnbnczazVOZEdRRUtFRGJBK2tnRmFKTFVzQ21I?=
 =?utf-8?B?anJoSTdmNlNuS0cyYkdmNDQ0aWFWTUlVTmw2T0ZOOG9uZU9Fc1NOS2FqdGN1?=
 =?utf-8?B?MVZYaDdDU2NlaVRrNUtZTGpONEJONVZwYkxuSGVrbXNiRisvUmcrSm5meHUw?=
 =?utf-8?B?M0dSK3M4aHRBL3lPenB3SmU3SlZ2bklOTU1kYm1zZnFiY0kxMEdQY1dMelJl?=
 =?utf-8?B?OXIraGo4ZHhOVm5RV2Vid05mYXJJQ0FkemNCbnlDWnVuRXZ5K1prY0lxZXdM?=
 =?utf-8?B?aUV5SFc3dGxkS04yVzJIenpVT2F2VGtaYXZFRjZWKzZ3OXpaODlwSGJTRC93?=
 =?utf-8?B?K3lhL3JrV3hxdGgzM1dTZm5YazVWMzZ1a2NuaXBDenNpZFQxT1QzRUJOS29C?=
 =?utf-8?B?UFV6ZnJRRU1kZ0Z1aXZjOHowM3FvSWxiYnBKai9qSzY2YUtIQWJ4dUxEdHc4?=
 =?utf-8?B?d3c1WHI5MGg4T3dVYndFMGpqUmJXS1ZaVkhORk93dkQ0Q0ZQZUZ4cXdnOStx?=
 =?utf-8?B?aHMrYlJoSGtKd3RUKzNpeS9aTW1SVzZZaUZOT2VNeWNsUUlCNUpkNHR4OUhh?=
 =?utf-8?B?K2c9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	RH1xFwTlQbjOz0kkYvEKzrAOPsz7R3pfdi7MXioJqZg9L2OkStL8KO3ozgmxM/BsjKvT8VDttS+IvePM9m0Ydv/ebYvkCQRcWWWTfUw+VLKblkoRiLzGuEDY1qTLBvxCNhOFLuTGGHG5OJdOxD0LsEodo9XXFKuZ32+4AQCkhjwPyN5H+t3f1ztDoUFCQnVoD4WP6tAxURr1ElJNzKshDtMwj9N66AKZT7mF+vHGeF5R53p+7t0Dz9mj3WdSxR9NJuC2Ax/ZOSy0zXexRMXdK4LyFt/ISottOYpvSMBduxiAi4Ir/ZkDYgu0Nyhqx9YuPxq79aBqCDvRRvY7emDo9wgW8yYo1rLl6Cqd2hqEv//R8+esnOPl86bKuKoM2TxkmEoGeljAAURXipSDibg6vzvS9ZVaEs88NqAAn1dNfTCX8dY0DPRGKNQy8VsY/j1Dp5fLBZgKGxTQ0HH16j0+YcLBK7LaKDjP6ZNd4EJFyKqTPkWpBriwqDmqpRLRKiD/MHRt6wdQymnXY3/aixq7392ojkRGMyutnADfcytSHpoLyZWR8fNh4hbFZ/e/oR1vcVZsQi2bKYfbzWdXLp+H2n6ryuHaVS0bsXwDXFNiDOo=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b1b80895-59b3-4eab-bf01-08dc6533756d
X-MS-Exchange-CrossTenant-AuthSource: MW5PR10MB5738.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2024 14:24:46.9301
 (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: 4qlHwzLTCN0BKnYhqxf/6tpTemsg2VZgTeKZCFua5xJ15Ug2e7wA7Lg6aYjbUh4/5S7ryxGVtSu1x9pAdyfVzlfk62TLdmNvM/6HjGzLjug=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4837
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-25_14,2024-04-25_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 mlxscore=0
 mlxlogscore=999 bulkscore=0 malwarescore=0 spamscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404250105
X-Proofpoint-GUID: N-T5G9uPnca4Jglm6GKuTBj9Aa42odzt
X-Proofpoint-ORIG-GUID: N-T5G9uPnca4Jglm6GKuTBj9Aa42odzt

On 4/25/24 9:17AM, Matthew Wilcox wrote:
> On Thu, Apr 25, 2024 at 11:10:38PM +0900, Jeongjun Park wrote:
>> Matthew Wilcox wrote:
>>> If that's the problem then the correct place to detect & reject this is
>>> during mount, not at inode free time.
>>
>> I fixed the patch as you said. If you patch in this way, the
>> file system will not be affected by the vulnerability at all
>> due to the code structure.
> 
> It should be checked earlier than this.

Right. This is preferable.

>  There's this code in
> dbMount().  Why isn't this catching it?

db_agstart is not checked in this code. That may be the bad data in this 
case.

There are probably dozens more sanity checks that are missing when data 
is first read from the disk.

Shaggy

> 
>          bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
>          if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
>              bmp->db_agl2size < 0) {
>                  err = -EINVAL;
>                  goto err_release_metapage;
>          }
> 
>          if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) {
>                  err = -EINVAL;
>                  goto err_release_metapage;
>          }
> 
> 
>> Thanks.
>>
>> ---
>>   fs/jfs/jfs_imap.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>> index 2ec35889ad24..ba0aa2f145cc 100644
>> --- a/fs/jfs/jfs_imap.c
>> +++ b/fs/jfs/jfs_imap.c
>> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>>   int diRead(struct inode *ip)
>>   {
>>   	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
>> -	int iagno, ino, extno, rc;
>> +	int iagno, ino, extno, rc, agno;
>>   	struct inode *ipimap;
>>   	struct dinode *dp;
>>   	struct iag *iagp;
>> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>>   
>>   	/* get the ag for the iag */
>>   	agstart = le64_to_cpu(iagp->agstart);
>> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
>> +	if(agno >= MAXAG || agno < 0)
>> +		return -EIO;
>>   
>>   	release_metapage(mp);
>>   
>> -- 
>> 2.34.1