Re: [PATCH] 9p: use kvzalloc for readdir buffer

[David Laight <david.laight.linux@gmail.com>]

On Wed, 15 Apr 2026 07:45:08 +0200
"Pierre Barre" <pierre@barre.sh> wrote:

> The readdir buffer is sized to msize, so kzalloc() can fail under
> fragmentation with a page allocation failure in v9fs_alloc_rdir_buf()
> / v9fs_dir_readdir_dotl().
> 
> The buffer is only a response sink and is never pack_sg_list()'d,
> so kvzalloc() is safe for all transports, unlike the fcall buffers
> fixed in e21d451a82f3.
> 
> Signed-off-by: Pierre Barre <pierre@barre.sh>
> ---
>  fs/9p/vfs_dir.c | 2 +-
>  net/9p/client.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
> index af7f72abbb76..487c177aae38 100644
> --- a/fs/9p/vfs_dir.c
> +++ b/fs/9p/vfs_dir.c
> @@ -70,7 +70,7 @@ static struct p9_rdir *v9fs_alloc_rdir_buf(struct file *filp, int buflen)
>         struct p9_fid *fid = filp->private_data;
> 
>         if (!fid->rdir)
> -               fid->rdir = kzalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL);
> +               fid->rdir = kvzalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL);

That code isn't really well thought out at all.
The default for buflen seems to be 128k + 24 - 24.
struct p9_rdir adds another 8 bytes, I'm not sure whether that doubles it to
128k or just adds an extra page (which might be 64k not 4k).
It also looks as though the user can specify an arbitrary buffers size (min 4k).
So who knows how big that (and other associated) buffers actually are.

The buffer seems to persist across readdir() system calls.
I can't see any code that might attempt to honour seekdir().
I hope there is a global lock that stops multiple threads issuing readdir()
on the same fd in parallel.

	David

>         return fid->rdir;
>  }
> 
> diff --git a/net/9p/client.c b/net/9p/client.c
> index f60d1d041adb..6d9b9054841e 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -765,7 +765,7 @@ static void p9_fid_destroy(struct p9_fid *fid)
>         spin_lock_irqsave(&clnt->lock, flags);
>         idr_remove(&clnt->fids, fid->fid);
>         spin_unlock_irqrestore(&clnt->lock, flags);
> -       kfree(fid->rdir);
> +       kvfree(fid->rdir);
>         kfree(fid);
>  }
> 
> --
> 2.51.0
>