linux-kernel - Re: [PATCH 2/2] fuse: Adjust readdir() buffer to requesting buffer size.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19df312f-06a2-4e71-960a-32bc952b0ed2@uls.co.za>
Date: Tue, 1 Apr 2025 17:03:51 +0200
From: Jaco Kroon <jaco@....co.za>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: bernd.schubert@...tmail.fm, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, christophe.jaillet@...adoo.fr,
 joannelkoong@...il.com, rdunlap@...radead.org, trapexit@...wn.link,
 david.laight.linux@...il.com
Subject: Re: [PATCH 2/2] fuse: Adjust readdir() buffer to requesting buffer
 size.

Hi,

On 2025/04/01 16:40, Miklos Szeredi wrote:
> On Tue, 1 Apr 2025 at 16:29, Jaco Kroon <jaco@....co.za> wrote:
>> After:
>>
>> getdents64(3, 0x7ffae8eed040 /* 276 entries */, 131072) = 6696
>> getdents64(3, 0x7ffae8eed040 /* 0 entries */, 131072) = 0
> This looks great.  But see below.
>
>> Signed-off-by: Jaco Kroon <jaco@....co.za>
>> ---
>>   fs/fuse/readdir.c | 19 +++++++++++++++----
>>   1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
>> index 17ce9636a2b1..a13534f411b4 100644
>> --- a/fs/fuse/readdir.c
>> +++ b/fs/fuse/readdir.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/posix_acl.h>
>>   #include <linux/pagemap.h>
>>   #include <linux/highmem.h>
>> +#include <linux/minmax.h>
>>
>>   static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
>>   {
>> @@ -337,11 +338,21 @@ static int fuse_readdir_uncached(struct file *file, struct dir_context *ctx)
>>          struct fuse_mount *fm = get_fuse_mount(inode);
>>          struct fuse_io_args ia = {};
>>          struct fuse_args_pages *ap = &ia.ap;
>> -       struct fuse_folio_desc desc = { .length = PAGE_SIZE };
>> +       struct fuse_folio_desc desc = { .length = ctx->count };
>>          u64 attr_version = 0, evict_ctr = 0;
>>          bool locked;
>> +       int order;
>>
>> -       folio = folio_alloc(GFP_KERNEL, 0);
>> +       desc.length = clamp(desc.length, PAGE_SIZE, fm->fc->max_pages << PAGE_SHIFT);
>> +       order = get_count_order(desc.length >> CONFIG_PAGE_SHIFT);
>> +
>> +       do {
>> +               folio = folio_alloc(GFP_KERNEL, order);
>> +               if (folio)
>> +                       break;
>> +               --order;
>> +               desc.length = PAGE_SIZE << order;
>> +       } while (order >= 0);
>>          if (!folio)
>>                  return -ENOMEM;
> Why not use kvmalloc instead?
Because fuse_simple_request via fuse_args_pages (ap) via fuse_io_args 
(ia) expects folios and changing that is more than what I'm capable off, 
and has larger overall impact.
> We could also implement allocation based on size of result in dev.c to
> optimize this, as most directories will be small, but that can be done
> later.

This indeed sounds interesting and would be great, but again, beyond 
what I'm capable of doing at this stage.

Great insights.

Thank you.

Kind regards,
Jaco