[PATCH 1/2] nsfs: fix u32-vs-bytes unit mismatch in nsfs_fh_to_dentry()

[PATCH 1/2] nsfs: fix u32-vs-bytes unit mismatch in nsfs_fh_to_dentry()

[Hui Peng]

In nsfs_fh_to_dentry(), both fh_len and NSFS_FID_SIZE_U32_LATEST (4) are
expressed in units of 4-byte u32 words rather than bytes, whereas
pointer arithmetic on (void *)fid and the byte count passed to
memchr_inv() are in bytes (NSFS_FILE_HANDLE_SIZE_LATEST = 16).

Passing (void *)fid + NSFS_FID_SIZE_U32_LATEST and
fh_len - NSFS_FID_SIZE_U32_LATEST to memchr_inv() inspects bytes
[4 .. fh_len) inside struct nsfs_file_handle (fid->ns_id and
fid->ns_type) instead of the trailing bytes [16 .. fh_len * 4) after
struct nsfs_file_handle. Consequently:
1. Valid zero-padded handles with handle_bytes >= 36 (fh_len >= 9) where
   fid->ns_type != 0 (at byte offset 8) are falsely rejected with
   -ESTALE.
2. Non-zero trailing garbage in bytes [16 .. fh_len * 4) is ignored when
   the upper 32 bits of fid->ns_id (bytes [4..7]) are zero.

Fix this by offsetting (void *)fid by NSFS_FILE_HANDLE_SIZE_LATEST (16)
and multiplying (fh_len - NSFS_FID_SIZE_U32_LATEST) by sizeof(u32).

Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
 fs/nsfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index c3b6ae765..a1842e12f 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -529,8 +529,8 @@
 
 	/* Check that any trailing bytes are zero. */
 	if ((fh_len > NSFS_FID_SIZE_U32_LATEST) &&
-	    memchr_inv((void *)fid + NSFS_FID_SIZE_U32_LATEST, 0,
-		       fh_len - NSFS_FID_SIZE_U32_LATEST))
+	    memchr_inv((void *)fid + NSFS_FILE_HANDLE_SIZE_LATEST, 0,
+		       (fh_len - NSFS_FID_SIZE_U32_LATEST) * sizeof(u32)))
 		return NULL;
 
 	switch (fh_type) {
-- 
2.43.0

[PATCH 2/2] nsfs: fix namespace reference leak on unsupported ns_type in nsfs_fh_to_dentry()

[Hui Peng]

In nsfs_fh_to_dentry(), ns_get_unless_inactive(ns) acquires an active
reference to ns before switching on ns->ns_type. Unlike the CLONE_NEWPID
error path and the owning_ns permission error path, the default: branch
returns ERR_PTR(-EOPNOTSUPP) without calling ns->ops->put(ns), leaking
the namespace reference.

Call ns->ops->put(ns) before returning ERR_PTR(-EOPNOTSUPP) in the
default: branch.

Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
 fs/nsfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index a1842e12f..e9cc09583 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -624,6 +624,7 @@
 		break;
 #endif
 	default:
+		ns->ops->put(ns);
 		return ERR_PTR(-EOPNOTSUPP);
 	}
 
-- 
2.43.0

[PATCH v2 1/2] nsfs: fix u32-vs-bytes unit mismatch in nsfs_fh_to_dentry()

[Hui Peng]

In nsfs_fh_to_dentry(), both fh_len and NSFS_FID_SIZE_U32_LATEST (4) are
expressed in units of 4-byte u32 words rather than bytes, whereas
pointer arithmetic on (void *)fid and the byte count passed to
memchr_inv() are in bytes (NSFS_FILE_HANDLE_SIZE_LATEST = 16).

Passing (void *)fid + NSFS_FID_SIZE_U32_LATEST and
fh_len - NSFS_FID_SIZE_U32_LATEST to memchr_inv() inspects bytes
[4 .. fh_len) inside struct nsfs_file_handle (fid->ns_id and
fid->ns_type) instead of the trailing bytes [16 .. fh_len * 4) after
struct nsfs_file_handle. Consequently:
1. Valid zero-padded handles with handle_bytes >= 36 (fh_len >= 9) where
   fid->ns_type != 0 (at byte offset 8) are falsely rejected with
   -ESTALE.
2. Non-zero trailing garbage in bytes [16 .. fh_len * 4) is ignored when
   the upper 32 bits of fid->ns_id (bytes [4..7]) are zero.

Fix this by offsetting (void *)fid by NSFS_FILE_HANDLE_SIZE_LATEST (16)
and multiplying (fh_len - NSFS_FID_SIZE_U32_LATEST) by sizeof(u32).

Fixes: 5222470b2fbb ("nsfs: support file handles")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v2: Add a Fixes: tag. nsfs_fh_to_dentry(), both NSFS_FID_SIZE_U32_*
constants and this comparison were all added together by 5222470b2fbb
("nsfs: support file handles"), first released in v6.18.

To be explicit about the impact, since a Fixes: tag will get this picked
up for stable: this is NOT a memory-safety bug. (void *)fid + 4 stays
well inside the 16-byte struct nsfs_file_handle, and fh_len - 4 is
smaller than the intended (fh_len - 4) * 4, so the existing scan is
strictly narrower than it should be. The consequences are a spurious
-ESTALE for handles with handle_bytes >= 36, and trailing non-zero bytes
not being rejected as intended.

 fs/nsfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index c3b6ae765..a1842e12f 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -529,8 +529,8 @@
 
 	/* Check that any trailing bytes are zero. */
 	if ((fh_len > NSFS_FID_SIZE_U32_LATEST) &&
-	    memchr_inv((void *)fid + NSFS_FID_SIZE_U32_LATEST, 0,
-		       fh_len - NSFS_FID_SIZE_U32_LATEST))
+	    memchr_inv((void *)fid + NSFS_FILE_HANDLE_SIZE_LATEST, 0,
+		       (fh_len - NSFS_FID_SIZE_U32_LATEST) * sizeof(u32)))
 		return NULL;
 
 	switch (fh_type) {
-- 
2.43.0

[PATCH v2 2/2] nsfs: fix namespace reference leak on unsupported ns_type in nsfs_fh_to_dentry()

[Hui Peng]

In nsfs_fh_to_dentry(), ns_get_unless_inactive(ns) acquires an active
reference to ns before switching on ns->ns_type. Unlike the CLONE_NEWPID
error path and the owning_ns permission error path, the default: branch
returns ERR_PTR(-EOPNOTSUPP) without calling ns->ops->put(ns), leaking
the namespace reference.

Call ns->ops->put(ns) before returning ERR_PTR(-EOPNOTSUPP) in the
default: branch.

Fixes: 5222470b2fbb ("nsfs: support file handles")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v2: Add a Fixes: tag. I blamed the default: label and its return
separately from patch 1/2 to be sure: both come from 5222470b2fbb
("nsfs: support file handles"), and the asymmetry is present in the
original version - the sibling CLONE_NEWPID and ns_capable() error paths
both call ns->ops->put(ns), only default: does not. The two later
commits touching this switch, 4055526d3574 ("ns: move ns type into
struct ns_common") and 3a18f809184b ("ns: add active reference count"),
only change the switch expression and how the reference is taken; they
do not add or remove a put.

Please note I have no reproducer for this one and I am not claiming it
is currently reachable: ns comes from ns_tree_lookup_rcu(), which only
returns namespaces present in the ns tree, and every namespace type that
can exist in a given config has a matching case, gated by the same
CONFIG_* symbols. So default: looks dead today and this is really an
error-path-symmetry / future-proofing fix rather than a live leak.
Treat it accordingly for stable - I would not object to dropping the
Fixes: tag to keep AUTOSEL away from it, or to folding the three error
paths into a common goto out_put; if you prefer that shape.

 fs/nsfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index a1842e12f..e9cc09583 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -624,6 +624,7 @@
 		break;
 #endif
 	default:
+		ns->ops->put(ns);
 		return ERR_PTR(-EOPNOTSUPP);
 	}
 
-- 
2.43.0