Message-Id: <20260206214613.work.184-kees@kernel.org>
Date: Fri,  6 Feb 2026 13:46:18 -0800
From: Kees Cook <kees@...nel.org>
To: "Theodore Ts'o" <tytso@....edu>
Cc: Kees Cook <kees@...nel.org>,
	李龙兴 <coregee2000@...il.com>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	Andy Shevchenko <andriy.shevchenko@...el.com>,
	linux-ext4@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: [PATCH ALTERNATIVE] ext4: Treat s_mount_opts and mount_opts as __nonstring

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

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 match the report today.)

Effectively revert 8ecb790ea8c3, 3db63d2c2d1d, and ee5a977b4e77, and fix
04a91570ac67 to treat s_mount_opts and ext4_tune_sb_params::mount_opts
as __nonstring.

Reported-by: 李龙兴 <coregee2000@...il.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@...nel.org>
---
Here's the alternative, though I have not heavily tested this. I'm hoping
the ext4 regression tests have images with 64 character s_mount_opts...

Cc: "Theodore Ts'o" <tytso@....edu>
Cc: Andreas Dilger <adilger.kernel@...ger.ca>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: <linux-ext4@...r.kernel.org>
---
 fs/ext4/ext4.h            |  2 +-
 include/uapi/linux/ext4.h |  2 +-
 fs/ext4/ioctl.c           | 12 ++++++------
 fs/ext4/super.c           |  5 ++---
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 56112f201cac..588d51a41c38 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1448,7 +1448,7 @@ struct ext4_super_block {
 	__le64	s_last_error_block;	/* block involved of last error */
 	__u8	s_last_error_func[32] __nonstring;	/* function where the error happened */
 #define EXT4_S_ERR_END offsetof(struct ext4_super_block, s_mount_opts)
-	__u8	s_mount_opts[64];
+	__u8	s_mount_opts[64] __nonstring;
 	__le32	s_usr_quota_inum;	/* inode for tracking user quota */
 	__le32	s_grp_quota_inum;	/* inode for tracking group quota */
 	__le32	s_overhead_clusters;	/* overhead blocks/clusters in fs */
diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
index 9c683991c32f..551373a775be 100644
--- a/include/uapi/linux/ext4.h
+++ b/include/uapi/linux/ext4.h
@@ -138,7 +138,7 @@ struct ext4_tune_sb_params {
 	__u32 clear_feature_compat_mask;
 	__u32 clear_feature_incompat_mask;
 	__u32 clear_feature_ro_compat_mask;
-	__u8  mount_opts[64];
+	__u8  mount_opts[64] __kernel_nonstring;
 	__u8  pad[68];
 };
 
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7ce0fc40aec2..9fd4118c9805 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1292,7 +1292,9 @@ static int ext4_ioctl_get_tune_sb(struct ext4_sb_info *sbi,
 	ret.raid_stripe_width = le32_to_cpu(es->s_raid_stripe_width);
 	ret.encoding = le16_to_cpu(es->s_encoding);
 	ret.encoding_flags = le16_to_cpu(es->s_encoding_flags);
-	strscpy_pad(ret.mount_opts, es->s_mount_opts);
+	memcpy_and_pad(ret.mount_opts, sizeof(ret.mount_opts),
+		       es->s_mount_opts,
+		       strnlen(es->s_mount_opts, sizeof(es->s_mount_opts)), 0);
 	ret.feature_compat = le32_to_cpu(es->s_feature_compat);
 	ret.feature_incompat = le32_to_cpu(es->s_feature_incompat);
 	ret.feature_ro_compat = le32_to_cpu(es->s_feature_ro_compat);
@@ -1353,7 +1355,9 @@ static void ext4_sb_setparams(struct ext4_sb_info *sbi,
 		es->s_encoding = cpu_to_le16(params->encoding);
 	if (params->set_flags & EXT4_TUNE_FL_ENCODING_FLAGS)
 		es->s_encoding_flags = cpu_to_le16(params->encoding_flags);
-	strscpy_pad(es->s_mount_opts, params->mount_opts);
+	memcpy_and_pad(es->s_mount_opts, sizeof(es->s_mount_opts),
+		       params->mount_opts,
+		       strnlen(params->mount_opts, sizeof(params->mount_opts)), 0);
 	if (params->set_flags & EXT4_TUNE_FL_EDIT_FEATURES) {
 		es->s_feature_compat |=
 			cpu_to_le32(params->set_feature_compat_mask);
@@ -1394,10 +1398,6 @@ static int ext4_ioctl_set_tune_sb(struct file *filp,
 	if (copy_from_user(&params, in, sizeof(params)))
 		return -EFAULT;
 
-	if (strnlen(params.mount_opts, sizeof(params.mount_opts)) ==
-	    sizeof(params.mount_opts))
-		return -E2BIG;
-
 	if ((params.set_flags & ~TUNE_OPS_SUPPORTED) != 0)
 		return -EOPNOTSUPP;
 
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 87205660c5d0..fe3a8e7c6f03 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2477,7 +2477,7 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 					struct ext4_fs_context *m_ctx)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	char s_mount_opts[64];
+	char s_mount_opts[sizeof(sbi->s_es->s_mount_opts) + 1];
 	struct ext4_fs_context *s_ctx = NULL;
 	struct fs_context *fc = NULL;
 	int ret = -ENOMEM;
@@ -2485,8 +2485,7 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	if (!sbi->s_es->s_mount_opts[0])
 		return 0;
 
-	if (strscpy_pad(s_mount_opts, sbi->s_es->s_mount_opts) < 0)
-		return -E2BIG;
+	memtostr(s_mount_opts, sbi->s_es->s_mount_opts);
 
 	fc = kzalloc(sizeof(struct fs_context), GFP_KERNEL);
 	if (!fc)
-- 
2.34.1

Powered by blists - more mailing lists