linux-kernel - Re: [PATCH] fuse: enable larger read buffers for readdir.

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] [day] [month] [year] [list]

Message-ID: <CAJfpegvAZ-A09VKMcKJF1NMcgMf7Jq6yoCQR8ixh2eme6LkOjw@mail.gmail.com>
Date:   Fri, 28 Jul 2023 10:42:59 +0200
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Bernd Schubert <bernd.schubert@...tmail.fm>
Cc:     Jaco Kroon <jaco@....co.za>,
        Antonio SJ Musumeci <trapexit@...wn.link>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fuse: enable larger read buffers for readdir.

On Thu, 27 Jul 2023 at 21:43, Bernd Schubert <bernd.schubert@...tmail.fm> wrote:
>
>
>
> On 7/27/23 21:21, Miklos Szeredi wrote:
> > On Wed, 26 Jul 2023 at 20:40, Jaco Kroon <jaco@....co.za> wrote:
> >
> >> Will look into FUSE_INIT.  The FUSE_INIT as I understand from what I've
> >> read has some expansion constraints or the structure is somehow
> >> negotiated.  Older clients in other words that's not aware of the option
> >> will follow some default.  For backwards compatibility that default
> >> should probably be 1 page.  For performance reasons it makes sense that
> >> this limit be larger.
> >
> > Yes, might need this for backward compatibility.  But perhaps a
> > feature flag is enough and the readdir buf can be limited to
> > fc->max_read.
>
> fc->max_read is set by default to ~0 and only set to something else when
> the max_read mount option is given? So typically that is a large value
> (UINT_MAX)?

That's fine.  It probably still makes sense to limit it to 128k, but
with the ctx->count patch it would be naturally limited by the size of
the userspace buffer.  There's really no downside to enabling a large
buffer, other than an unlikely regression in userspace.    If server
wants to return less entries, it still can.  Unlike plain reads,
filling the buffer to the fullest extent isn't required for readdir.

So the buffer size calculation can be somthing like:

init:
#define FUSE_READDIR_MAX (128*1024)
fc->max_readdir = PAGE_SIZE;
if (flags & FUSE_LARGE_READDIR)
   fc->max_readdir = min(fc->max_read, FUSE_READDIR_MAX);

[...]
readdir:
   bufsize = min(fc->max_readdir, max(ctx->count, PAGE_SIZE));

Thanks,
Miklos