[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202602061229.D90563C500@keescook>
Date: Fri, 6 Feb 2026 12:36:29 -0800
From: Kees Cook <kees@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: 李龙兴 <coregee2000@...il.com>,
Theodore Ts'o <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
linux-ext4@...r.kernel.org, syzkaller@...glegroups.com,
andy@...nel.org, akpm@...ux-foundation.org,
linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org
Subject: Re: [Kernel Bug] WARNING in ext4_fill_super
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
Powered by blists - more mailing lists