[PATCH] ext4: wipe moved dirents with their real length

[PATCH] ext4: wipe moved dirents with their real length

[Adriano Cordova]

After copying a dirent to the new block, dx_move_dirents() wipes the
source entry using the on-disk rec_len.  Use the length already
computed for the copy instead because the on-disk rec_len is untrusted
and a corrupt value makes the memset() run out of bounds.

Reported-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd
Tested-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 fs/ext4/namei.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a6386c1d237f..71e9e7c9a7fd 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1860,10 +1860,8 @@ dx_move_dirents(struct inode *dir, char *from, char *to,
 
 		/* wipe dir_entry excluding the rec_len field */
 		de->inode = 0;
-		memset(&de->name_len, 0, ext4_rec_len_from_disk(de->rec_len,
-								blocksize) -
-					 offsetof(struct ext4_dir_entry_2,
-								name_len));
+		memset(&de->name_len, 0, rec_len -
+		       offsetof(struct ext4_dir_entry_2, name_len));
 
 		map++;
 		to += rec_len;
-- 
2.51.0

Re: [PATCH] ext4: wipe moved dirents with their real length

[Jan Kara]

On Thu 01-10-26 21:41:27, Adriano Cordova wrote:
> After copying a dirent to the new block, dx_move_dirents() wipes the
> source entry using the on-disk rec_len.  Use the length already
> computed for the copy instead because the on-disk rec_len is untrusted
> and a corrupt value makes the memset() run out of bounds.
> 
> Reported-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd
> Tested-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>

Not that this would be wrong per se but the explanation doesn't really
satisfy me :) If the rec_len on disk is wrong, we have a larger problem
with corrupted source directory and we shouldn't have reached
dx_move_dirents() in the first place. So how come directory entry
validation in dx_make_map() didn't catch the corruption? Did syzkaller
somehow managed to corrupt the directory entry between dx_make_map() and
dx_move_dirents()? If yes, that is the bug that needs fixing, not trying to
suck up corrupted directory entries in dx_move_dirents()...

								Honza

> ---
>  fs/ext4/namei.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index a6386c1d237f..71e9e7c9a7fd 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1860,10 +1860,8 @@ dx_move_dirents(struct inode *dir, char *from, char *to,
>  
>  		/* wipe dir_entry excluding the rec_len field */
>  		de->inode = 0;
> -		memset(&de->name_len, 0, ext4_rec_len_from_disk(de->rec_len,
> -								blocksize) -
> -					 offsetof(struct ext4_dir_entry_2,
> -								name_len));
> +		memset(&de->name_len, 0, rec_len -
> +		       offsetof(struct ext4_dir_entry_2, name_len));
>  
>  		map++;
>  		to += rec_len;
> -- 
> 2.51.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: wipe moved dirents with their real length

[Adriano Córdova]

El vie, 2 oct 2026 a las 9:45, Jan Kara (<jack@suse.cz>) escribió:
>
> On Thu 01-10-26 21:41:27, Adriano Cordova wrote:
> > After copying a dirent to the new block, dx_move_dirents() wipes the
> > source entry using the on-disk rec_len.  Use the length already
> > computed for the copy instead because the on-disk rec_len is untrusted
> > and a corrupt value makes the memset() run out of bounds.
> >
> > Reported-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd
> > Tested-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
> > Signed-off-by: Adriano Cordova <adrianox@gmail.com>
>
> Not that this would be wrong per se but the explanation doesn't really
> satisfy me :) If the rec_len on disk is wrong, we have a larger problem
> with corrupted source directory and we shouldn't have reached
> dx_move_dirents() in the first place. So how come directory entry
> validation in dx_make_map() didn't catch the corruption? Did syzkaller
> somehow managed to corrupt the directory entry between dx_make_map() and
> dx_move_dirents()? If yes, that is the bug that needs fixing, not trying to
> suck up corrupted directory entries in dx_move_dirents()...
>
>                                                                 Honza
>
> > ---
> >  fs/ext4/namei.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index a6386c1d237f..71e9e7c9a7fd 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -1860,10 +1860,8 @@ dx_move_dirents(struct inode *dir, char *from, char *to,
> >
> >               /* wipe dir_entry excluding the rec_len field */
> >               de->inode = 0;
> > -             memset(&de->name_len, 0, ext4_rec_len_from_disk(de->rec_len,
> > -                                                             blocksize) -
> > -                                      offsetof(struct ext4_dir_entry_2,
> > -                                                             name_len));
> > +             memset(&de->name_len, 0, rec_len -
> > +                    offsetof(struct ext4_dir_entry_2, name_len));
> >
> >               map++;
> >               to += rec_len;
> > --
> > 2.51.0
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Hi Jan,

Right. The syzbot C repro works by corrupting the block bitmap of the
in-disk filesystem, so it
has nothing to do with dirents. I will send a v2 with a proposed sanity check.

Adriano