fs/ext4/namei.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
The rec_len field in the directory entry has to be a multiple of 4. A
corrupted filesystem image can be used to hit a BUG() in
ext4_rec_len_to_disk(), called from make_indexed_dir().
------------[ cut here ]------------
kernel BUG at fs/ext4/ext4.h:2413!
...
RIP: 0010:make_indexed_dir+0x53f/0x5f0
...
Call Trace:
<TASK>
? add_dirent_to_buf+0x1b2/0x200
ext4_add_entry+0x36e/0x480
ext4_add_nondir+0x2b/0xc0
ext4_create+0x163/0x200
path_openat+0x635/0xe90
do_filp_open+0xb4/0x160
? __create_object.isra.0+0x1de/0x3b0
? _raw_spin_unlock+0x12/0x30
do_sys_openat2+0x91/0x150
__x64_sys_open+0x6c/0xa0
do_syscall_64+0x3c/0x80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
The fix simply adds a call to ext4_check_dir_entry() to validate the
directory entry, returning -EFSCORRUPTED if the entry is invalid.
CC: stable@vger.kernel.org
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216540
Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
* Changes since v1:
As suggested by Ted, I've removed the incorrect 'de->rec_len' check from
previous version and replaced it with a call to ext4_check_dir_entry()
instead, which is a much more complete verification.
fs/ext4/namei.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3a31b662f661..ed76e89ffbe9 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2254,8 +2254,16 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
memset(de, 0, len); /* wipe old data */
de = (struct ext4_dir_entry_2 *) data2;
top = data2 + len;
- while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top)
+ while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top) {
+ if (ext4_check_dir_entry(dir, NULL, de, bh2, data2, len,
+ (data2 + (blocksize - csum_size) -
+ (char *) de))) {
+ brelse(bh2);
+ brelse(bh);
+ return -EFSCORRUPTED;
+ }
de = de2;
+ }
de->rec_len = ext4_rec_len_to_disk(data2 + (blocksize - csum_size) -
(char *) de, blocksize);
On Wed, 12 Oct 2022 14:13:30 +0100, Luís Henriques wrote:
> The rec_len field in the directory entry has to be a multiple of 4. A
> corrupted filesystem image can be used to hit a BUG() in
> ext4_rec_len_to_disk(), called from make_indexed_dir().
>
> ------------[ cut here ]------------
> kernel BUG at fs/ext4/ext4.h:2413!
> ...
> RIP: 0010:make_indexed_dir+0x53f/0x5f0
> ...
> Call Trace:
> <TASK>
> ? add_dirent_to_buf+0x1b2/0x200
> ext4_add_entry+0x36e/0x480
> ext4_add_nondir+0x2b/0xc0
> ext4_create+0x163/0x200
> path_openat+0x635/0xe90
> do_filp_open+0xb4/0x160
> ? __create_object.isra.0+0x1de/0x3b0
> ? _raw_spin_unlock+0x12/0x30
> do_sys_openat2+0x91/0x150
> __x64_sys_open+0x6c/0xa0
> do_syscall_64+0x3c/0x80
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> [...]
Applied, thanks!
[1/1] ext4: fix BUG_ON() when directory entry has invalid rec_len
commit: 17a0bc9bd697f75cfdf9b378d5eb2d7409c91340
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
Grr, looks like I accidentally reused a 'git send-email' from shell
history which had a '--in-reply-to' in it. Please ignore and sorry about
that. I've just resent a new email.
Cheers,
--
Luís
On Wed, Oct 12, 2022 at 02:13:30PM +0100, Luís Henriques wrote:
> The rec_len field in the directory entry has to be a multiple of 4. A
> corrupted filesystem image can be used to hit a BUG() in
> ext4_rec_len_to_disk(), called from make_indexed_dir().
>
> ------------[ cut here ]------------
> kernel BUG at fs/ext4/ext4.h:2413!
> ...
> RIP: 0010:make_indexed_dir+0x53f/0x5f0
> ...
> Call Trace:
> <TASK>
> ? add_dirent_to_buf+0x1b2/0x200
> ext4_add_entry+0x36e/0x480
> ext4_add_nondir+0x2b/0xc0
> ext4_create+0x163/0x200
> path_openat+0x635/0xe90
> do_filp_open+0xb4/0x160
> ? __create_object.isra.0+0x1de/0x3b0
> ? _raw_spin_unlock+0x12/0x30
> do_sys_openat2+0x91/0x150
> __x64_sys_open+0x6c/0xa0
> do_syscall_64+0x3c/0x80
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> The fix simply adds a call to ext4_check_dir_entry() to validate the
> directory entry, returning -EFSCORRUPTED if the entry is invalid.
>
> CC: stable@vger.kernel.org
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216540
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
> * Changes since v1:
>
> As suggested by Ted, I've removed the incorrect 'de->rec_len' check from
> previous version and replaced it with a call to ext4_check_dir_entry()
> instead, which is a much more complete verification.
>
> fs/ext4/namei.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 3a31b662f661..ed76e89ffbe9 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2254,8 +2254,16 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> memset(de, 0, len); /* wipe old data */
> de = (struct ext4_dir_entry_2 *) data2;
> top = data2 + len;
> - while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top)
> + while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top) {
> + if (ext4_check_dir_entry(dir, NULL, de, bh2, data2, len,
> + (data2 + (blocksize - csum_size) -
> + (char *) de))) {
> + brelse(bh2);
> + brelse(bh);
> + return -EFSCORRUPTED;
> + }
> de = de2;
> + }
> de->rec_len = ext4_rec_len_to_disk(data2 + (blocksize - csum_size) -
> (char *) de, blocksize);
>
On Wed, Oct 12, 2022 at 02:16:42PM +0100, Luís Henriques wrote: > Grr, looks like I accidentally reused a 'git send-email' from shell > history which had a '--in-reply-to' in it. Please ignore and sorry about > that. I've just resent a new email. No worries! The --in-reply-to wasn't actually a problem, since b4 generally will do the right thing (and sometimes humans prefer the in-reply-to since they can more easily see the patch that it is replacing/obsoleting). b4 can sometimes get confused when a patch series gets split, and both parts of the patch series are in a reply-to mail thread to the original patch series, since if it can't use the -vn+1 hueristic or the "subject line has stayed the same but has a newer date" hueristic, it falls back to "latest patch in the mail thread". So if there are two "valid" patches or patch series in an e-mail thread, b4 -c (--check-newer-revisions) can get confused. But even in that case, that it's more a minor annoyance than anything else. So in the future, don't feel that you need to resend a patch if there's an incorrect/older --in-reply-to; it's not a big deal. Cheers, and thanks! - Ted
On Wed, Oct 12, 2022 at 10:21:39AM -0400, Theodore Ts'o wrote: > On Wed, Oct 12, 2022 at 02:16:42PM +0100, Luís Henriques wrote: > > Grr, looks like I accidentally reused a 'git send-email' from shell > > history which had a '--in-reply-to' in it. Please ignore and sorry about > > that. I've just resent a new email. > > No worries! The --in-reply-to wasn't actually a problem, since b4 > generally will do the right thing (and sometimes humans prefer the > in-reply-to since they can more easily see the patch that it is > replacing/obsoleting). > > b4 can sometimes get confused when a patch series gets split, and both > parts of the patch series are in a reply-to mail thread to the > original patch series, since if it can't use the -vn+1 hueristic or > the "subject line has stayed the same but has a newer date" hueristic, > it falls back to "latest patch in the mail thread". So if there are > two "valid" patches or patch series in an e-mail thread, b4 -c > (--check-newer-revisions) can get confused. But even in that case, > that it's more a minor annoyance than anything else. > > So in the future, don't feel that you need to resend a patch if > there's an incorrect/older --in-reply-to; it's not a big deal. Great, I haven't yet included b4 in my workflow so, to be honest, I didn't really thought about that tool being confused. What really made me resend the patch was that I used the *wrong message-ID in the "--in-reply-to"! And that thread already had a v2 patch, which would could easily confuse humans. Hopefully, b4 won't be confused by that either. Cheers, -- Luís
© 2016 - 2026 Red Hat, Inc.