open_by_handle_at(2) gets the wrong answer for a namespace file handle longer than struct nsfs_file_handle: a 16-byte handle padded with zeros to 48 bytes or more, which is what the comment above the check promises to accept, is refused with -ESTALE, while a 20-byte handle whose extra word is garbage is accepted. Reproduced on v7.0 and on mainline with the handle of /proc/self/ns/net from name_to_handle_at(AT_SYMLINK_FOLLOW); no privilege is needed, as nsfs_fh_to_dentry() admits the caller's own namespaces. Since v6.18. nsfs_fh_to_dentry() receives fh_len in 32-bit words, like every fh_to_dentry(), and NSFS_FID_SIZE_U32_LATEST is the struct size in words (4). The check that the bytes past the struct are zero adds that word count to a byte pointer and passes the word difference to memchr_inv() as a byte count: it inspects bytes 4 .. fh_len - 1 of the handle instead of bytes 16 .. 4 * fh_len - 1. Up to a 44-byte handle that range lies inside ns_id, ns_type and ns_inum, so the outcome depends on the values of those fields (a padded net namespace handle passes because the upper bytes of its ns_id and ns_type are zero) and the real trailing bytes are never looked at; from 48 bytes on the range always covers a non-zero field byte and every padded handle is refused. Start NSFS_FILE_HANDLE_SIZE_LATEST bytes into the handle and check (fh_len - NSFS_FID_SIZE_U32_LATEST) * sizeof(u32) bytes. Tested with a program that encodes /proc/self/ns/net and opens the handle with handle_bytes 16, 20, 36, 48 and 128 (zero padding), and 20 and 48 with a non-zero last byte. Before: 48 and 128 refused, the 20-byte garbage handle accepted; after: every padded handle opens, both garbage handles are refused. Fixes: 5222470b2fbb ("nsfs: support file handles") Assisted-by: LLM Signed-off-by: Nickolai Zeldovich --- Notes: The arithmetic keeps the two existing constants: NSFS_FILE_HANDLE_SIZE_LATEST is the struct size in bytes, NSFS_FID_SIZE_U32_LATEST the same in words, and fh_len is in words as exportfs passes it (handle_bytes >> 2). pidfs has no trailing-bytes check, so nothing else uses this pattern. Reproducer (any kernel >= 6.18 with CONFIG_FHANDLE, no privilege): #define _GNU_SOURCE #include #include #include #include #include #define MAX_HANDLE_SZ 128 static int fails; static void try(int mntfd, struct file_handle *fh, unsigned int bytes, unsigned int poison_at, int expect_ok, const char *what) { int fd; memset(fh->f_handle + 16, 0, MAX_HANDLE_SZ - 16); if (poison_at) fh->f_handle[poison_at] = 0xff; fh->handle_bytes = bytes; fd = open_by_handle_at(mntfd, fh, O_RDONLY); if ((fd >= 0) != expect_ok) fails++; printf("%s handle_bytes=%u %s -> %s\n", (fd >= 0) == expect_ok ? "ok " : "FAIL", bytes, what, fd >= 0 ? "opened" : strerror(errno)); if (fd >= 0) close(fd); } int main(void) { char buf[sizeof(struct file_handle) + MAX_HANDLE_SZ] = {}; struct file_handle *fh = (struct file_handle *)buf; int mntfd, mnt_id; fh->handle_bytes = MAX_HANDLE_SZ; mntfd = open("/proc/self/ns/net", O_RDONLY); if (mntfd < 0 || name_to_handle_at(AT_FDCWD, "/proc/self/ns/net", fh, &mnt_id, AT_SYMLINK_FOLLOW)) { perror("name_to_handle_at"); return 2; } try(mntfd, fh, 16, 0, 1, "as encoded"); try(mntfd, fh, 20, 0, 1, "one zero word of padding"); try(mntfd, fh, 48, 0, 1, "eight zero words of padding"); try(mntfd, fh, 128, 0, 1, "zero-padded to MAX_HANDLE_SZ"); try(mntfd, fh, 20, 19, 0, "non-zero byte 19 (past the struct)"); try(mntfd, fh, 48, 47, 0, "non-zero byte 47 (past the struct)"); printf("%s\n", fails ? "FAIL" : "PASS"); return fails != 0; } gcc -O2 -Wall -o nsfs_fh_pad nsfs_fh_pad.c && ./nsfs_fh_pad before: ok handle_bytes=16 as encoded -> opened ok handle_bytes=20 one zero word of padding -> opened FAIL handle_bytes=48 eight zero words of padding -> Stale file handle FAIL handle_bytes=128 zero-padded to MAX_HANDLE_SZ -> Stale file handle FAIL handle_bytes=20 non-zero byte 19 (past the struct) -> opened ok handle_bytes=48 non-zero byte 47 (past the struct) -> Stale file handle FAIL after: all six ok, PASS fs/nsfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/nsfs.c b/fs/nsfs.c index c3b6ae76594a..0ef4e1455b7a 100644 --- a/fs/nsfs.c +++ b/fs/nsfs.c @@ -529,8 +529,8 @@ static struct dentry *nsfs_fh_to_dentry(struct super_block *sb, struct fid *fh, /* 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) { base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da -- 2.53.0