Re: [PATCH 1/1] ext4: fix potential negative array index in do_split()

From: Eric Sandeen <sandeen@sandeen.net>
To: Lukas Czerner <lczerner@redhat.com>, Eric Sandeen <sandeen@redhat.com>
Cc: "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 1/1] ext4: fix potential negative array index in do_split()
Date: Fri, 19 Jun 2020 08:44:19 -0500	[thread overview]
Message-ID: <be37a8fe-78a8-bcb6-8d30-d975fb1ec080@sandeen.net> (raw)
In-Reply-To: <20200619111631.ugx7sdpci32ohgir@work>

On 6/19/20 6:16 AM, Lukas Czerner wrote:

>> The other possibility is that map[i].size is not right and indeed there
>> seems to be a bug in dx_make_map()
>>
>> map_tail->size = le16_to_cpu(de->rec_len);
>>
>> should be
>>
>> map_tail->size = ext4_rec_len_from_disk(de->rec_len, blocksize));
>>
>> right ? Otherwise with large enough records the size will be smaller
>> than it really is.
>>
>> A quick look at fs/ext4/namei.c reveals couple of places there rec_len
>> is used without the conversion and we should check whether it needs
>> fixing.
>>
>> -Lukas
> 
> And indeed the following patch seems to have fixed the issue we were
> seeing. Eric I think that this might be a proper fix. But we still need
> to check the other uses of rec_len to make sure it's ok as well.
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 94ec882..5509fdc 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1068,7 +1068,7 @@ static int dx_make_map(struct ext4_dir_entry_2 *de, unsigned blocksize,
>                         map_tail--;
>                         map_tail->hash = h.hash;
>                         map_tail->offs = ((char *) de - base)>>2;
> -                       map_tail->size = le16_to_cpu(de->rec_len);
> +                       map_tail->size = ext4_rec_len_from_disk(le16_to_cpu(de->rec_len), blocksize);

That isn't right, ext4_rec_len_from_disk /takes/ an __le16 :)

-                       map_tail->size = le16_to_cpu(de->rec_len);
+                       map_tail->size = ext4_rec_len_from_disk(de->rec_len), blocksize);

would be more correct, but won't matter for PAGE_SIZE < 65536 right?

-Eric