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

Hi David,

Perhaps what you describe can explain what I was seeing there: https://lore.kernel.org/v9fs/[email protected]/

Thanks,
Pierre

On Wed, Apr 15, 2026, at 11:01, David Laight wrote:
> On Wed, 15 Apr 2026 07:45:08 +0200
> "Pierre Barre" <[email protected]> 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 <[email protected]>
>> ---
>>  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
>>