linux-kernel - Re: [PATCH v6 2/2] fuse: add new function to invalidate cache for all inodes

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: <87a5ah521y.fsf@igalia.com>
Date: Wed, 19 Feb 2025 16:31:21 +0000
From: Luis Henriques <luis@...lia.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: Dave Chinner <david@...morbit.com>,  Bernd Schubert <bschubert@....com>,
  Alexander Viro <viro@...iv.linux.org.uk>,  Christian Brauner
 <brauner@...nel.org>,  Jan Kara <jack@...e.cz>,  Matt Harvey
 <mharvey@...ptrading.com>,  linux-fsdevel@...r.kernel.org,
  linux-kernel@...r.kernel.org,  Valentin Volkl <valentin.volkl@...n.ch>,
  Laura Promberger <laura.promberger@...n.ch>
Subject: Re: [PATCH v6 2/2] fuse: add new function to invalidate cache for
 all inodes

On Wed, Feb 19 2025, Miklos Szeredi wrote:

> On Wed, 19 Feb 2025 at 12:23, Luis Henriques <luis@...lia.com> wrote:
>
>> +static int fuse_notify_update_epoch(struct fuse_conn *fc)
>> +{
>> +       struct fuse_mount *fm;
>> +       struct inode *inode;
>> +
>> +       inode = fuse_ilookup(fc, FUSE_ROOT_ID, &fm);
>> +       if (!inode) || !fm)
>> +               return -ENOENT;
>> +
>> +       iput(inode);
>> +       atomic_inc(&fc->epoch);
>> +       shrink_dcache_sb(fm->sb);
>
> This is just an optimization and could be racy, kicking out valid
> cache (harmlessly of course).  I'd leave it out of the first version.

OK, will do.

> There could be more than one fuse_mount instance.  Wondering if epoch
> should be per-fm not per-fc...

Good question.  Because the cache is shared among the several fuse_mount
instances the epoch may eventually affect all of them even if it's a
per-fm attribute.  But on the other hand, different mounts could focus on
a different set of filesystem subtrees so... yeah, I'll probably leave it
in fc for now while thinking about it some more.

>> @@ -204,6 +204,12 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
>>         int ret;
>>
>>         inode = d_inode_rcu(entry);
>> +       if (inode) {
>> +               fm = get_fuse_mount(inode);
>> +               if (entry->d_time < atomic_read(&fm->fc->epoch))
>> +                       goto invalid;
>> +       }
>
> Negative dentries need to be invalidated too.

Ack.

>> @@ -446,6 +452,12 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
>>                 goto out_err;
>>
>>         entry = newent ? newent : entry;
>> +       if (inode) {
>> +               struct fuse_mount *fm = get_fuse_mount(inode);
>> +               entry->d_time = atomic_read(&fm->fc->epoch);
>> +       } else {
>> +               entry->d_time = 0;
>> +       }
>
> Again, should do the same for positive and negative dentries.
>
> Need to read out fc->epoch before sending the request to the server,
> otherwise might get a stale dentry with an updated epoch.

Ah, good point.

> This also needs to be done in fuse_create_open(), create_new_entry()
> and fuse_direntplus_link().

Yeah I suspected there were a few other places where this would be
required.  I'll look closer into that.

Thanks a lot for your feedback, Miklos.  I'll work on this new approach,
so that I can send a real patch soon.

Cheers,
-- 
Luís