linux-kernel - Re: [RFC PATCH v2] fuse: add optional workqueue to periodically invalidate expired dentries

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: <87tt5o7kpk.fsf@igalia.com>
Date: Tue, 13 May 2025 13:34:47 +0100
From: Luis Henriques <luis@...lia.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: Bernd Schubert <bernd@...ernd.com>,  Laura Promberger
 <laura.promberger@...n.ch>,  Dave Chinner <david@...morbit.com>,  Matt
 Harvey <mharvey@...ptrading.com>,  linux-fsdevel@...r.kernel.org,
  linux-kernel@...r.kernel.org,  kernel-dev@...lia.com
Subject: Re: [RFC PATCH v2] fuse: add optional workqueue to periodically
 invalidate expired dentries

Hi Miklos,

On Tue, May 13 2025, Miklos Szeredi wrote:

> On Tue, 15 Apr 2025 at 15:38, Luis Henriques <luis@...lia.com> wrote:
>
>> +inval_wq=N
>> +  Enable a workqueue that will periodically invalidate dentries that
>> +  have expired.  'N' is a value in seconds and has to be bigger than
>> +  5 seconds.
>> +
>
> I don't think a mount option is needed.  Perhaps a module option knob
> instead is sufficient?

Sure, that should do the trick.  It'll still be set to zero by default
(i.e. no periodic invalidation), and it won't be possible to tune it per
mount.  Which is probably the right thing to do.

>> +static void fuse_dentry_tree_add_node(struct dentry *dentry)
>> +{
>> +       struct fuse_conn *fc = get_fuse_conn_super(dentry->d_sb);
>> +       struct dentry_node *dn, *cur;
>> +       struct rb_node **p, *parent = NULL;
>> +       bool start_work = false;
>> +
>> +       if (!fc->inval_wq)
>> +               return;
>> +
>> +       dn = kmalloc(sizeof(*dn), GFP_KERNEL);
>> +       if (!dn)
>> +               return;
>> +       dn->dentry = dget(dentry);
>
> A dentry ref without a vfsmount ref is generally bad idea.
>
> Instead of acquiring a ref, the lifetime of dn should be tied to that
> of the dentry (hook into ->d_prune).
>
> So just put the rb_node in fuse_dentry and get rid of the "#if
> BITS_PER_LONG >= 64" optimization.

OK, this probably makes more sense.  I'll have a closer look at the
details, but it seems to be a better option.  Thanks a lot for the
suggestion.  I'll work on that for v3.

Cheers,
-- 
Luís