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

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

[Kees Cook]

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.)

The other option is to go back in time and mark both s_mount_opts and
mount_opts as __nonstring and switch to using memcpy_and_pad() to copy
them around between userspace and kernel and disk instead of making them
C strings.

What do the ext4 regression tests expect for s_mount_opts? Is there a
test for a non-terminated s_mount_opts in an image?

Reported-by: 李龙兴 <coregee2000@gmail.com>
Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/
Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: <linux-ext4@vger.kernel.org>
---
 fs/ext4/super.c | 7 +++++++
 1 file changed, 7 insertions(+)

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;
 
-- 
2.34.1

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

[Darrick J. Wong]

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.

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.

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

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"?

--D

> The other option is to go back in time and mark both s_mount_opts and
> mount_opts as __nonstring and switch to using memcpy_and_pad() to copy
> them around between userspace and kernel and disk instead of making them
> C strings.
> 
> What do the ext4 regression tests expect for s_mount_opts? Is there a
> test for a non-terminated s_mount_opts in an image?
> 
> Reported-by: 李龙兴 <coregee2000@gmail.com>
> Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/
> Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> Cc: <linux-ext4@vger.kernel.org>
> ---
>  fs/ext4/super.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> 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;
>  
> -- 
> 2.34.1
> 
> 

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

[Kees Cook]

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

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

[Andy Shevchenko]

On Fri, Feb 06, 2026 at 01:56:52PM -0800, Kees Cook wrote:
> 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.)

I agree with your patch, so here a couple of remarks:

- the questions you asked in the commit messages probably need to be replaced
with the summary of what Darrick said

- should the not NUL-terminated string be a fatal error? Maybe we can tell the user
that it's truncated and go on with it? Dunno (on the first glance this sounds better
from user perspective, but it may lead to a weird results if the cut string will be
parsed perfectly and give something dangerous to the data on the volume.

-- 
With Best Regards,
Andy Shevchenko

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

[Fedor Pchelkin]

Kees Cook wrote:
> 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.)
> 

[...]

> Reported-by: 李龙兴 <coregee2000@gmail.com>
> Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/
> Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")

Hi there,

[ I'd better be Cc'ed as the author of the commit in Fixes ]

The mentioned reports are for v6.18.2 kernel while ee5a977b4e77 ("ext4:
fix string copying in parse_apply_sb_mount_options()") landed in v6.18.3.
Back at the time I've tested the patch with different bogus s_mount_opts
values and the fortify warnings should have been gone.

I don't think there is an error in ee5a977b4e77 unless these warnings
actually appear on the latest kernels with ee5a977b4e77 applied.

> @@ -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;
> +	}

strscpy_pad() returns -E2BIG if the source string was truncated.  This
happens for the above condition as well - the last byte is truncated and
replaced with a NUL-terminator.

The check at 3db63d2c2d1d ("ext4: check if mount_opts is NUL-terminated in
ext4_ioctl_set_tune_sb()") was done in that manner as there is currently
no way to propagate strscpy_pad() return value up from ext4_sb_setparams().
So the string is independently checked inside ext4_ioctl_set_tune_sb()
directly.


As for the 64/65 byte length part, now the rationale of the checks works
as Darrick Wong described at the other part of this thread and corresponds
to how relevant userspace stuff treats the s_mount_opts field: the buffer
is at most 63 payload characters long + NUL-terminator.  Jan Kara also
shared similar thoughts during the discussion of ee5a977b4e77 [1].

[1]: https://lore.kernel.org/linux-ext4/yq6rbx54jt4btntsh37urd6u63wwcd3lyhovbrm6w7occaveea@riljfkx5jmhi/

> +
>  	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
>  		return -E2BIG;
>  
> -- 
> 2.34.1

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

[Kees Cook]

On Mon, Feb 09, 2026 at 08:27:50PM +0300, Fedor Pchelkin wrote:
> Kees Cook wrote:
> > 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.)
> > 
> 
> [...]
> 
> > Reported-by: 李龙兴 <coregee2000@gmail.com>
> > Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/
> > Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")
> 
> Hi there,
> 
> [ I'd better be Cc'ed as the author of the commit in Fixes ]

Agreed! Sorry I missed adding you to Cc.

> The mentioned reports are for v6.18.2 kernel while ee5a977b4e77 ("ext4:
> fix string copying in parse_apply_sb_mount_options()") landed in v6.18.3.
> Back at the time I've tested the patch with different bogus s_mount_opts
> values and the fortify warnings should have been gone.

Ah-ha! Okay, thank you for catching this versioning issue. I had been
scratching my head over how it could have been the same warning. This
report is effectively a duplicate of the report you fixed with
ee5a977b4e77.

> I don't think there is an error in ee5a977b4e77 unless these warnings
> actually appear on the latest kernels with ee5a977b4e77 applied.
> 
> > @@ -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;
> > +	}
> 
> strscpy_pad() returns -E2BIG if the source string was truncated.  This
> happens for the above condition as well - the last byte is truncated and
> replaced with a NUL-terminator.

Yeah, I've double-checked this now. The second half of the overflow
check in the fortified strnlen eluded by eyes when I went through this
originally. Thanks for sanity checking this!

> The check at 3db63d2c2d1d ("ext4: check if mount_opts is NUL-terminated in
> ext4_ioctl_set_tune_sb()") was done in that manner as there is currently
> no way to propagate strscpy_pad() return value up from ext4_sb_setparams().
> So the string is independently checked inside ext4_ioctl_set_tune_sb()
> directly.
> 
> 
> As for the 64/65 byte length part, now the rationale of the checks works
> as Darrick Wong described at the other part of this thread and corresponds
> to how relevant userspace stuff treats the s_mount_opts field: the buffer
> is at most 63 payload characters long + NUL-terminator.  Jan Kara also
> shared similar thoughts during the discussion of ee5a977b4e77 [1].
> 
> [1]: https://lore.kernel.org/linux-ext4/yq6rbx54jt4btntsh37urd6u63wwcd3lyhovbrm6w7occaveea@riljfkx5jmhi/

Okay, great. I figure I can do two things:

1) rework this patch with adjusted commit log to reflect the notes
   raised so far, so that we reject mounts that lack a NUL-terminated
   s_mount_opts (as silent truncation may induce an unintended option
   string, e.g. "...,journal_path=/dev/sda2" into "...,journal_path=/dev/sda"
   or something weird like that).

2) Leave everything as-is, live with above corner case since it should
   be unreachable with userspace tooling as they have always existed.

I'm fine either way! :)

-Kees

-- 
Kees Cook

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

[Darrick J. Wong]

On Mon, Feb 09, 2026 at 12:00:36PM -0800, Kees Cook wrote:
> On Mon, Feb 09, 2026 at 08:27:50PM +0300, Fedor Pchelkin wrote:
> > Kees Cook wrote:
> > > 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.)
> > > 
> > 
> > [...]
> > 
> > > Reported-by: 李龙兴 <coregee2000@gmail.com>
> > > Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/
> > > Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()")
> > 
> > Hi there,
> > 
> > [ I'd better be Cc'ed as the author of the commit in Fixes ]
> 
> Agreed! Sorry I missed adding you to Cc.
> 
> > The mentioned reports are for v6.18.2 kernel while ee5a977b4e77 ("ext4:
> > fix string copying in parse_apply_sb_mount_options()") landed in v6.18.3.
> > Back at the time I've tested the patch with different bogus s_mount_opts
> > values and the fortify warnings should have been gone.
> 
> Ah-ha! Okay, thank you for catching this versioning issue. I had been
> scratching my head over how it could have been the same warning. This
> report is effectively a duplicate of the report you fixed with
> ee5a977b4e77.
> 
> > I don't think there is an error in ee5a977b4e77 unless these warnings
> > actually appear on the latest kernels with ee5a977b4e77 applied.
> > 
> > > @@ -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;
> > > +	}
> > 
> > strscpy_pad() returns -E2BIG if the source string was truncated.  This
> > happens for the above condition as well - the last byte is truncated and
> > replaced with a NUL-terminator.
> 
> Yeah, I've double-checked this now. The second half of the overflow
> check in the fortified strnlen eluded by eyes when I went through this
> originally. Thanks for sanity checking this!
> 
> > The check at 3db63d2c2d1d ("ext4: check if mount_opts is NUL-terminated in
> > ext4_ioctl_set_tune_sb()") was done in that manner as there is currently
> > no way to propagate strscpy_pad() return value up from ext4_sb_setparams().
> > So the string is independently checked inside ext4_ioctl_set_tune_sb()
> > directly.
> > 
> > 
> > As for the 64/65 byte length part, now the rationale of the checks works
> > as Darrick Wong described at the other part of this thread and corresponds
> > to how relevant userspace stuff treats the s_mount_opts field: the buffer
> > is at most 63 payload characters long + NUL-terminator.  Jan Kara also
> > shared similar thoughts during the discussion of ee5a977b4e77 [1].
> > 
> > [1]: https://lore.kernel.org/linux-ext4/yq6rbx54jt4btntsh37urd6u63wwcd3lyhovbrm6w7occaveea@riljfkx5jmhi/
> 
> Okay, great. I figure I can do two things:
> 
> 1) rework this patch with adjusted commit log to reflect the notes
>    raised so far, so that we reject mounts that lack a NUL-terminated
>    s_mount_opts (as silent truncation may induce an unintended option
>    string, e.g. "...,journal_path=/dev/sda2" into "...,journal_path=/dev/sda"
>    or something weird like that).
> 
> 2) Leave everything as-is, live with above corner case since it should
>    be unreachable with userspace tooling as they have always existed.
> 
> I'm fine either way! :)

I'd pick #1, unless someone knows of a userspace program that could have
set a 64-byte s_mount_ops string with no null terminator.  I didn't find
any, but there are many implementations of ext4 out there. :/

(and yes, it's better to reject an unterminated s_mount_opts than
accidentally point the kernel at the wrong block device)

--D

> -Kees
> 
> -- 
> Kees Cook
> 

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

[Fedor Pchelkin]

On Mon, 09. Feb 21:01, Darrick J. Wong wrote:
> On Mon, Feb 09, 2026 at 12:00:36PM -0800, Kees Cook wrote:
> > Okay, great. I figure I can do two things:
> > 
> > 1) rework this patch with adjusted commit log to reflect the notes
> >    raised so far, so that we reject mounts that lack a NUL-terminated
> >    s_mount_opts (as silent truncation may induce an unintended option
> >    string, e.g. "...,journal_path=/dev/sda2" into "...,journal_path=/dev/sda"
> >    or something weird like that).
> > 
> > 2) Leave everything as-is, live with above corner case since it should
> >    be unreachable with userspace tooling as they have always existed.
> > 
> > I'm fine either way! :)
> 
> I'd pick #1, unless someone knows of a userspace program that could have
> set a 64-byte s_mount_ops string with no null terminator.  I didn't find
> any, but there are many implementations of ext4 out there. :/
> 
> (and yes, it's better to reject an unterminated s_mount_opts than
> accidentally point the kernel at the wrong block device)

If I understand the issue correctly, it's already being rejected with the
existing check:

	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
		return -E2BIG;

If the source string is truncated at least by one symbol (which is the
case for unterminated string), strscpy_pad() returns -E2BIG and the mount
fails.

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

[Kees Cook]

On Tue, Feb 10, 2026 at 12:03:35PM +0300, Fedor Pchelkin wrote:
> If I understand the issue correctly, it's already being rejected with the
> existing check:
> 
> 	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
> 		return -E2BIG;
> 
> If the source string is truncated at least by one symbol (which is the
> case for unterminated string), strscpy_pad() returns -E2BIG and the mount
> fails.

Agreed. Sorry for the noise! Things appear to be fine as-is. I had
missed the version tested in the original report.

-- 
Kees Cook