[PATCH 0/2] fs: two mount API fixes

[PATCH 0/2] fs: two mount API fixes

[fujo]

Two mount API edge cases reported by shaos, both reproduced on
7.3.0-rc7 (58161ead2) with a small syscall probe run as /init under
QEMU with KASAN:

 - statmount(2) with STATMOUNT_MNT_UIDMAP on a mount that is not
   idmapped and a short buffer copied the reserved empty string at
   offset 512, one byte past the declared bufsize, and returned
   success.

 - listmount(2) and listns(2) with a U64_MAX cursor wrapped around to
   zero and restarted the enumeration at the first entry.

Probe output before:

  uidmap buf=100  ret=0   size=101 byte512=0
  lm cursor=MAX   ret=1   id0=2147483651
  ln cursor=MAX   ret=8   id0=1 id1=2

after:

  uidmap buf=100  ret=-75 (EOVERFLOW) byte512=170
  lm cursor=MAX   ret=-22 (EINVAL)
  ln cursor=MAX   ret=-22 (EINVAL)

The in-tree statmount_test (17/17), statmount_test_ns (2/2) and
listmount_test (2/2) pass with both patches applied.

Thanks,
fujo

[PATCH 1/2] fs: statmount: don't copy strings past the caller's buffer

[fujo]

statmount(2) collects all strings in a kernel buffer and copies them to
the user buffer after struct statmount.  copy_statmount_to_user() copies
seq->count bytes at buf + sizeof(struct statmount), but only
statmount_string() checks whether they fit: it returns early when the
producer emitted nothing, before the size check.

If no producer emits anything -- for example STATMOUNT_MNT_UIDMAP on a
mount that is not idmapped -- the reserved empty string is the only
thing left in seq->buf.  statmount(2) then copies that single NUL at
offset 512 even when the caller passed a smaller bufsize, and returns
success with size = 101 for bufsize = 100.  The write is beyond the
buffer the caller declared; if the following page is unmapped the
syscall fails with EFAULT instead.  The same request with a producer
that does emit data correctly returns EOVERFLOW.

Validate that the string area fits into s->bufsize before copying it
and return EOVERFLOW otherwise.

Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/all/20261020110747.959829-1-shawnq1@163.com/
Signed-off-by: fujo <hooyan2024@gmail.com>
---
 fs/namespace.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index c30f06374..c8aceeff6 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5831,8 +5831,18 @@ static int copy_statmount_to_user(struct kstatmount *s)
 	char __user *str = ((char __user *)s->buf) + sizeof(*sm);
 	size_t copysize = min_t(size_t, s->bufsize, sizeof(*sm));
 
-	if (seq->count && copy_to_user(str, seq->buf, seq->count))
-		return -EFAULT;
+	if (seq->count) {
+		/*
+		 * The string area starts after the struct and must fit into the
+		 * caller-provided buffer.  A request that emitted no strings
+		 * still leaves the reserved empty string in seq->buf.
+		 */
+		if (s->bufsize < sizeof(*sm) ||
+		    seq->count > s->bufsize - sizeof(*sm))
+			return -EOVERFLOW;
+		if (copy_to_user(str, seq->buf, seq->count))
+			return -EFAULT;
+	}
 
 	/* Return the number of bytes copied to the buffer */
 	sm->size = copysize + seq->count;
-- 
2.53.0

[PATCH 2/2] fs: reject U64_MAX cursors in listmount() and listns()

[fujo]

listmount(2) and listns(2) treat their cursor as the last ID returned by
the previous invocation and look up the next entry at cursor + 1.  For
U64_MAX that addition wraps to zero, so the enumeration silently
restarts at the first visible entry instead of reporting that there are
no more entries.

listmount(LSMT_ROOT, param = U64_MAX) returned the same first mount ID
as param = 0, while param = U64_MAX - 1 returned no entries.  listns()
repeated the first page of namespace IDs for a U64_MAX cursor;
U64_MAX - 1 returned ENOENT.

U64_MAX can never be an ID returned by a previous call: mount IDs are
allocated above MNT_UNIQUE_ID_OFFSET and are strictly increasing, and
namespace IDs come from an increasing 64-bit counter.  Reject it as an
invalid cursor, like the existing check for reserved mount IDs, so that
the wrap cannot happen.

Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/all/20261020110732.959552-1-shawnq1@163.com/
Signed-off-by: fujo <hooyan2024@gmail.com>
---
 fs/namespace.c  | 9 +++++++--
 kernel/nstree.c | 3 +++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index c8aceeff6..6806745a3 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -6292,8 +6292,13 @@ static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *
 	u64 last_mnt_id = kreq->param;
 	struct mnt_namespace *ns;
 
-	/* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
-	if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
+	/*
+	 * The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1.  A
+	 * U64_MAX cursor is invalid as well: advancing it by one would wrap
+	 * around to zero and restart the enumeration.
+	 */
+	if (last_mnt_id != 0 &&
+	    (last_mnt_id <= MNT_UNIQUE_ID_OFFSET || last_mnt_id == U64_MAX))
 		return -EINVAL;
 
 	kls->last_mnt_id = last_mnt_id;
diff --git a/kernel/nstree.c b/kernel/nstree.c
index 831f279d1..39978b56b 100644
--- a/kernel/nstree.c
+++ b/kernel/nstree.c
@@ -450,6 +450,9 @@ static int copy_ns_id_req(const struct ns_id_req __user *req,
 static inline int prepare_klistns(struct klistns *kls, struct ns_id_req *kreq,
 				  u64 __user *ns_ids, size_t nr_ns_ids)
 {
+	/* A U64_MAX cursor would wrap when advanced. */
+	if (kreq->ns_id == U64_MAX)
+		return -EINVAL;
 	kls->last_ns_id = kreq->ns_id;
 	kls->user_ns_id = kreq->user_ns_id;
 	kls->nr_ns_ids	= nr_ns_ids;
-- 
2.53.0