Hi,
Thanks for the patch! A few comments below.
On 6/25/25 8:20 PM, zhangjian wrote:
> 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.
No signed-off-by?
> ---
> fs/nfs/export.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> index e9c233b6f..e0e77f8ca 100644
> --- a/fs/nfs/export.c
> +++ b/fs/nfs/export.c
> @@ -65,8 +65,8 @@ nfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
> int fh_len, int fh_type)
> {
> struct nfs_fattr *fattr = NULL;
> - struct nfs_fh *server_fh = nfs_exp_embedfh(fid->raw);
I think we still need to call nfs_exp_embedfh() here, otherwise this pointer
will be unset during your "check for user input size" check down below.
> - size_t fh_size = offsetof(struct nfs_fh, data) + server_fh->size;
> + struct nfs_fh *server_fh;
> + size_t fh_size;
> const struct nfs_rpc_ops *rpc_ops;
> struct dentry *dentry;
> struct inode *inode;
We also have "int len = EMBED_FH_OFF + XDR_QUADLEN(fh_size);" being initialized
here. You should probably also remove this initialization since you're doing it
again down below.
> @@ -74,6 +74,14 @@ nfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
> u32 *p = fid->raw;
> int ret;
>
> + /* check for user input size */
> + if ((char*)server_fh <= (char*)p
^ Trailing whitespace> + || (int)((u32*)server_fh - (u32*)p + 1) < fh_len)
> + return ERR_PTR(-EINVAL);
^^^^^^^^ Trailing whitespace
Thanks,
Anna
> +
> + 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;
