Hi, Zhang Jian
server_fh is obtained via (struct nfs_fh *)(p + EMBED_FH_OFF). Shouldn't
the condition (char*)server_fh <= (char*)p always be false?
Additionally, (u32*)server_fh - (u32*)p + 1 appears to be a fixed value.
Why use such an expression?
Finally, fh_len is derived from user-provided handle->handle_bytes. Is
this reliable?
By the way, you shouldn't add Anna and Benjamin's Reviewd-by, because they
haven't seen this version of your changes, and they also have some
comments on your previous version of changes. Also, Jeff only gave
Reviewd-by for your previous version of changes, and your new version of
changes is different from the previous one, so you shouldn't add it.
Thanks,
Lingfeng.
在 2025/6/27 4:20, zhangjian 写道:
Syzkaller found an slab-out-of-bounds in nfs_fh_to_dentry when the memory
of server_fh is not passed from user space. So I add a check for input size.
Log is snipped as following:
==================================================================
BUG: KASAN: slab-out-of-bounds in nfs_fh_to_dentry+0x4ad/0x6d0 fs/nfs/export.c:70
Read of size 2 at addr ffff888100b9ffd4 by task syz-executor301/755
CPU: 1 PID: 755 Comm: syz-executor301 Tainted: G W 5.10.0 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:82 [inline]
dump_stack+0x107/0x167 lib/dump_stack.c:123
print_address_description.constprop.0+0x1e/0x280 mm/kasan/report.c:400
__kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
kasan_report+0x3a/0x50 mm/kasan/report.c:585
nfs_fh_to_dentry+0x4ad/0x6d0 fs/nfs/export.c:70
exportfs_decode_fh_raw+0x128/0x680 fs/exportfs/expfs.c:436
exportfs_decode_fh+0x3d/0x90 fs/exportfs/expfs.c:575
do_handle_to_path fs/fhandle.c:152 [inline]
handle_to_path fs/fhandle.c:207 [inline]
do_handle_open+0x2c3/0x8d0 fs/fhandle.c:223
do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x67/0xd1
==================================================================
Signed-off-by: zhangjian <zhangjian496@xxxxxxxxxx>
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
Reviewed-by: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
Reviewed-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
---
fs/nfs/export.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/export.c b/fs/nfs/export.c
index e9c233b6f..565e01788 100644
--- a/fs/nfs/export.c
+++ b/fs/nfs/export.c
@@ -66,14 +66,21 @@ nfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
{
struct nfs_fattr *fattr = NULL;
struct nfs_fh *server_fh = nfs_exp_embedfh(fid->raw);
- size_t fh_size = offsetof(struct nfs_fh, data) + server_fh->size;
+ size_t fh_size;
const struct nfs_rpc_ops *rpc_ops;
struct dentry *dentry;
struct inode *inode;
- int len = EMBED_FH_OFF + XDR_QUADLEN(fh_size);
+ int len;
u32 *p = fid->raw;
int ret;
+ /* check for user input size */
+ if ((char*)server_fh <= (char*)p || (int)((u32*)server_fh - (u32*)p + 1) < fh_len)
+ return ERR_PTR(-EINVAL);
+
+ fh_size = offsetof(struct nfs_fh, data) + server_fh->size;
+ len = EMBED_FH_OFF + XDR_QUADLEN(fh_size);
+
/* NULL translates to ESTALE */
if (fh_len < len || fh_type != len)
return NULL;
