Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

[Dave Kleikamp <dave.kleikamp@oracle.com>]

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