Re: [PATCH] ext4: Reject on-disk mount options with missing NUL-terminator

[Kees Cook <kees@kernel.org>]

On Fri, Feb 06, 2026 at 01:45:02PM -0800, Darrick J. Wong wrote:
> On Fri, Feb 06, 2026 at 01:27:03PM -0800, Kees Cook wrote:
> > When mounting an ext4 filesystem, the on-disk superblock's s_mount_opts
> > field (which stores default mount options set via tune2fs) is read and
> > parsed. Unlike userspace-provided mount options which are validated by
> > the VFS layer before reaching the filesystem, the on-disk s_mount_opts
> > is read directly from the disk buffer without NUL-termination validation.
> > 
> > The two option paths use the same parser but arrive differently:
> > 
> >   Userspace mount options:
> >     VFS -> ext4_parse_param()
> > 
> >   On-disk default options:
> >     parse_apply_sb_mount_options() -> parse_options() -> ext4_parse_param()
> > 
> > When s_mount_opts lacks NUL-termination, strscpy_pad()'s internal
> > fortified strnlen() detects reading beyond the 64-byte field, triggering
> > an Oops:
> > 
> >   strnlen: detected buffer overflow: 65 byte read of buffer size 64
> >   WARNING: CPU: 0 PID: 179 at lib/string_helpers.c:1035 __fortify_report+0x5a/0x100
> >   ...
> >   Call Trace:
> >    strnlen+0x71/0xa0 lib/string.c:155
> >    sized_strscpy+0x48/0x2a0 lib/string.c:298
> >    parse_apply_sb_mount_options+0x94/0x4a0 fs/ext4/super.c:2486
> >    __ext4_fill_super+0x31d6/0x51b0 fs/ext4/super.c:5306
> >    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
> >    do_new_mount fs/namespace.c:3718
> >    path_mount+0x7b9/0x23a0 fs/namespace.c:4028
> >    ...
> > 
> > Reject the mount with an error instead. While there is an existing similar
> > check in the ioctl path (which validates userspace input before writing
> > TO disk), this check validates data read FROM disk before parsing.
> > 
> > The painful history here is:
> > 
> > 8b67f04ab9de ("ext4: Add mount options in superblock")
> >   s_mount_opts is created and treated as __nonstring: kstrndup is used to
> >   make sure all 64 potential characters are available for use (i.e. up
> >   to 65 bytes may be allocated).
> > 
> > 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
> >   Created ext4_tune_sb_params::mount_opts as 64 bytes in size but
> >   incorrectly treated it and s_mount_opts as a C strings (it used
> >   strscpy_pad() to copy between them).
> > 
> > 8ecb790ea8c3 ("ext4: avoid potential buffer over-read in parse_apply_sb_mount_options()")
> >   As a prerequisite to the ioctl treating s_mount_opts as a C string, this
> >   attempted to switch to using strscpy_pad() with a 65 byte destination
> >   for the case of an unterminated s_mount_opts. (But strscpy_pad() will
> >   fail due to the over-read of s_mount_opts by strnlen().)
> > 
> > 3db63d2c2d1d ("ext4: check if mount_opts is NUL-terminated in ext4_ioctl_set_tune_sb()")
> >   As a continuation of trying to solve the 64/65 mismatch, this started
> >   enforcing a 63 character limit (i.e. 64 bytes total) to incoming values
> >   from userspace to the ioctl API. (But did not check s_mount_opts coming
> >   from disk.)
> > 
> > ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")
> >   Notices the loud failures of strscpy_pad() introduced by 8ecb790ea8c3,
> >   and attempted to silence them by making the destination 64 and rejecting
> >   too-long strings from the on-disk copy of s_mount_opts, but didn't
> >   actually solve it at all, since the problem was always the over-read
> >   of the source seen by strnlen(). (Note that the report quoted in this
> >   commit exactly matches the report today.)
> 
> Looking through the history of tune2fs, I see v1.42 has commit
> 9a976ac732a7e9 ("tune2fs: Fix mount_opts handling"), which always set
> byte 63 to a null, which implies that s_mount_opts is supposed to be a
> 64-byte field that contains a null-terminated string.  It also doesn't
> appear that slack space after the null terminator is set to any
> particular value.

Okay, so the userspace side is treating it as a C string. Then ignore my
"PATCH ALTERNATIVE" I just sent. ;)

> In e2fsprogs v1.41.13, commit 9345f02671fd39 ("tune2fs, debugfs,
> libext2fs: Add support for ext4 default mount options") added the
> ability to set s_mount_opts, and it rejects any string whose length
> is greater than or equal to 64 bytes.  This validation is still in
> place.

Great, thanks for tracking that down. That looks to be Dec 2010.

> My guess is that the ext4 code should go back to requiring a null
> terminated string, and the __nonstring annotations dropped.

It does already; no __nonstring annotations currently exist for
s_mount_opts. The trouble was that it was originally treated as
effectively __nonstring between 8b67f04ab9de (v2.6.36, Oct 2010) and
04a91570ac67 (v6.18, Nov 2025).

> I guess the hard part is, were there any filesystems written out with a
> 64-byte unterminated s_mount_opts?  It looks like 04a91570ac67 used
> strscpy_pad to copy the string to the superblock and that function
> always zero-terminates the destination, so perhaps we can stick with
> "the ondisk superblock s_mount_opts is always a null terminated string"?

Yeah, if the userspace tools always wrote images with a NUL-terminated
s_mount_opts, then it should be perfectly sane to reject mounts that
don't follow that. (i.e. this patch.)

-- 
Kees Cook