Re: [Kernel Bug] WARNING in ext4_fill_super

Re: [Kernel Bug] WARNING in ext4_fill_super

[Kees Cook]

On Fri, Feb 06, 2026 at 05:12:34PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 06, 2026 at 05:08:19PM +0200, Andy Shevchenko wrote:
> > On Fri, Feb 06, 2026 at 04:20:15PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 02, 2026 at 12:19:45PM +0800, 李龙兴 wrote:
> > > > Dear Linux kernel developers and maintainers,
> > > > 
> > > > We would like to report a new kernel bug found by our tool. The issue
> > > > is a WARNING in ext4_fill_super. Details are as follows.
> > > 
> > > First of all, the warning appears in parse_apply_sb_mount_options().
> [...]
> Actually, the documentation says that strscpy*() must be used against C-strings.
> This can explain the bug, id est the given string in mount options is not
> NUL-terminated. That's where bug may come from. So, the Q is why is mount options
> not NUL-terminated when it comes to ext4_fill_super()?

parse_apply_sb_mount_options(...):
	...
        char s_mount_opts[64];
	...
        if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
                return -E2BIG;

Is sbi->s_es->s_mount_opts expected to be a C string? If not, this
strscpy_pad() should likely be memtostr_pad(). (s_mount_opts is expected
to be a C string based on its use with later C string API calls.)

It seems like s_mount_opts is expected to be a C string, I can see it
being used that way in lots of other places, e.g.:

fs/ext4/ioctl.c:        strscpy_pad(ret.mount_opts, es->s_mount_opts);
fs/ext4/ioctl.c:        strscpy_pad(es->s_mount_opts, params->mount_opts);
fs/ext4/super.c:        if (!sbi->s_es->s_mount_opts[0])
fs/ext4/super.c:        if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)

I can't tell where es->s_mount_opts comes from originally?

This one:

fs/ext4/ioctl.c:        strscpy_pad(es->s_mount_opts, params->mount_opts);

comes through ext4_ioctl_set_tune_sb() which has:

        if (strnlen(params.mount_opts, sizeof(params.mount_opts)) ==
            sizeof(params.mount_opts))
                return -E2BIG;

So it's already checked for, and suggests it must be NUL-terminated.

> > > > loop4: detected capacity change from 0 to 514
> > > > ------------[ cut here ]------------
> > > > strnlen: detected buffer overflow: 65 byte read of buffer size 64
> > > > WARNING: CPU: 0 PID: 12320 at lib/string_helpers.c:1035
> > > > __fortify_report+0x9c/0xd0 lib/string_helpers.c:1035
> > > > [...]
> > > > Call Trace:
> > > >  <TASK>
> > > >  __fortify_panic+0x23/0x30 lib/string_helpers.c:1042
> > > >  strnlen include/linux/fortify-string.h:235 [inline]
> > > >  sized_strscpy include/linux/fortify-string.h:309 [inline]
> > > >  parse_apply_sb_mount_options fs/ext4/super.c:2486 [inline]
> > > >  __ext4_fill_super fs/ext4/super.c:5306 [inline]
> > > >  ext4_fill_super+0x3972/0xaf70 fs/ext4/super.c:5736
> > > >  get_tree_bdev_flags+0x38c/0x620 fs/super.c:1698
> > > >  vfs_get_tree+0x8e/0x340 fs/super.c:1758
> > > >  fc_mount fs/namespace.c:1199 [inline]
> > > >  do_new_mount_fc fs/namespace.c:3642 [inline]

So, something via do_new_mount_fc? Probably:

static int ext4_fill_super(struct super_block *sb, struct fs_context *fc)

which calls __ext4_fill_super(), and eventually
parse_apply_sb_mount_options(). Which depends on:

        struct ext4_fs_context *ctx = fc->fs_private;

But I can't figure out where that comes from. Seems like fs_parse(), but
I don't see where mount option strings would come through...

Anyone more familiar with this know?

-Kees

-- 
Kees Cook

Re: [Kernel Bug] WARNING in ext4_fill_super

[Kees Cook]

On Fri, Feb 06, 2026 at 11:29:11AM -0800, Kees Cook wrote:
> But I can't figure out where that comes from. Seems like fs_parse(), but
> I don't see where mount option strings would come through...

Oh! This is coming directly from disk. So we need an in-place sanity
check. How about this?


diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 87205660c5d0..9ad6005615d8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2485,6 +2485,13 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	if (!sbi->s_es->s_mount_opts[0])
 		return 0;
 
+	if (strnlen(sbi->s_es->s_mount_opts, sizeof(sbi->s_es->s_mount_opts)) ==
+	    sizeof(sbi->s_es->s_mount_opts)) {
+		ext4_msg(sb, KERN_ERR,
+			 "Mount options in superblock are not NUL-terminated");
+		return -EINVAL;
+	}
+
 	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
 		return -E2BIG;
 

-- 
Kees Cook

Re: [Kernel Bug] WARNING in ext4_fill_super

[Kees Cook]

On Fri, Feb 06, 2026 at 11:53:09AM -0800, Kees Cook wrote:
> On Fri, Feb 06, 2026 at 11:29:11AM -0800, Kees Cook wrote:
> > But I can't figure out where that comes from. Seems like fs_parse(), but
> > I don't see where mount option strings would come through...
> 
> Oh! This is coming directly from disk. So we need an in-place sanity
> check. How about this?
> 
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 87205660c5d0..9ad6005615d8 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2485,6 +2485,13 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
>  	if (!sbi->s_es->s_mount_opts[0])
>  		return 0;
>  
> +	if (strnlen(sbi->s_es->s_mount_opts, sizeof(sbi->s_es->s_mount_opts)) ==
> +	    sizeof(sbi->s_es->s_mount_opts)) {
> +		ext4_msg(sb, KERN_ERR,
> +			 "Mount options in superblock are not NUL-terminated");
> +		return -EINVAL;
> +	}
> +
>  	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
>  		return -E2BIG;

Oh, wait. I see these commits now:

8ecb790ea8c3 ("ext4: avoid potential buffer over-read in parse_apply_sb_mount_options()")
ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")

Ugh, the history is that fundamentally s_mount_opts should be
__nonstring. Old code handled this fine (by using kstrndup), but when
ioctl get/set was added in commit 04a91570ac67 ("ext4: implemet new
ioctls to set and get superblock parameters"), s_mount_opts started
being treated as a C string, which would lead to over-reads (due to lack
of NUL-termination).

So, should on-disk s_mount_opts be required to be NUL-terminated? I
would argue yes, since right now a mount of such a thing will crash with
the reported failure in this thread. So likely, my proposed fix is the
best option?

-Kees

-- 
Kees Cook