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
