[PATCH 1/2] fs: preserve ACL_DONT_CACHE state in forget_cached_acl()

[PATCH 1/2] fs: preserve ACL_DONT_CACHE state in forget_cached_acl()

[Amir Goldstein]

The ACL_DONT_CACHE state is meant to be a constant state for the inode
for filesystems that want to opt out of posix acl caching.

Commit facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
used this facility to opt out of posix acl caching for fuse inodes with
fuse server that does not negotiate FUSE_POSIX_ACL (fc->posix_acl).

The commit also takes care to gate the forget_all_cached_acls() call in
fuse_set_acl() on fc->posix_acl because there is no need for it, but
there are other placed in fuse code which call forget_all_cached_acls()
unconditional to fc->posix_acl and those cause the loss of the
ACL_DONT_CACHE state.

This is not only a functional bug. Properly timed, a get_acl() from this
fuse filesystem can return a stale cached value, as was observed in tests,
because set_acl() does not invalidate the unintentional acl cache.

We could fix this in fuse, but it actually makes no sense for the vfs
helper forget_cached_acl() to invalidate the ACL_DONT_CACHE state, so
let it not do that to fix fuse and future users of ACL_DONT_CACHE.

Fixes: facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
Cc: stable@vger.kernel.org
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/posix_acl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index b4bfe4ddf64ea..3dc62c1c27087 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -93,6 +93,13 @@ static void __forget_cached_acl(struct posix_acl **p)
 {
 	struct posix_acl *old;
 
+	/*
+	 * ACL_DONT_CACHE is expected to be a "const" value and xchg it with
+	 * ACL_NOT_CACHED would enable acl caching for the inode -
+	 * clearly not what the caller has intended.
+	 */
+	if (READ_ONCE(*p) == ACL_DONT_CACHE)
+		return;
 	old = xchg(p, ACL_NOT_CACHED);
 	if (!is_uncached_acl(old))
 		posix_acl_release(old);
-- 
2.54.0

Re: [PATCH 1/2] fs: preserve ACL_DONT_CACHE state in forget_cached_acl()

[Christian Brauner]

> The ACL_DONT_CACHE state is meant to be a constant state for the inode
> for filesystems that want to opt out of posix acl caching.
> 
> Commit facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> used this facility to opt out of posix acl caching for fuse inodes with
> fuse server that does not negotiate FUSE_POSIX_ACL (fc->posix_acl).
> 
> The commit also takes care to gate the forget_all_cached_acls() call in
> fuse_set_acl() on fc->posix_acl because there is no need for it, but
> there are other placed in fuse code which call forget_all_cached_acls()
> unconditional to fc->posix_acl and those cause the loss of the
> ACL_DONT_CACHE state.
> 
> This is not only a functional bug. Properly timed, a get_acl() from this
> fuse filesystem can return a stale cached value, as was observed in tests,
> because set_acl() does not invalidate the unintentional acl cache.
> 
> We could fix this in fuse, but it actually makes no sense for the vfs
> helper forget_cached_acl() to invalidate the ACL_DONT_CACHE state, so
> let it not do that to fix fuse and future users of ACL_DONT_CACHE.
> 
> Fixes: facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> Cc: stable@vger.kernel.org
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index b4bfe4ddf64e..3dc62c1c2708 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -93,6 +93,13 @@ static void __forget_cached_acl(struct posix_acl **p)
>  {
>  	struct posix_acl *old;
>  
> +	/*
> +	 * ACL_DONT_CACHE is expected to be a "const" value and xchg it with
> +	 * ACL_NOT_CACHED would enable acl caching for the inode -
> +	 * clearly not what the caller has intended.
> +	 */
> +	if (READ_ONCE(*p) == ACL_DONT_CACHE)
> +		return;

Still on vacation this week but I took a glimpse:

If this isn't what the caller intended, having ACL_DONT_CACHE end up
should be treated like a bug. So shouldn't this then be a WARN_ON_ONCE()
and return?

-- 
Christian Brauner <brauner@kernel.org>

Re: [PATCH 1/2] fs: preserve ACL_DONT_CACHE state in forget_cached_acl()

[Amir Goldstein]

On Wed, Jul 15, 2026 at 11:02 AM Christian Brauner <brauner@kernel.org> wrote:
>
> > The ACL_DONT_CACHE state is meant to be a constant state for the inode
> > for filesystems that want to opt out of posix acl caching.
> >
> > Commit facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> > used this facility to opt out of posix acl caching for fuse inodes with
> > fuse server that does not negotiate FUSE_POSIX_ACL (fc->posix_acl).
> >
> > The commit also takes care to gate the forget_all_cached_acls() call in
> > fuse_set_acl() on fc->posix_acl because there is no need for it, but
> > there are other placed in fuse code which call forget_all_cached_acls()
> > unconditional to fc->posix_acl and those cause the loss of the
> > ACL_DONT_CACHE state.
> >
> > This is not only a functional bug. Properly timed, a get_acl() from this
> > fuse filesystem can return a stale cached value, as was observed in tests,
> > because set_acl() does not invalidate the unintentional acl cache.
> >
> > We could fix this in fuse, but it actually makes no sense for the vfs
> > helper forget_cached_acl() to invalidate the ACL_DONT_CACHE state, so
> > let it not do that to fix fuse and future users of ACL_DONT_CACHE.
> >
> > Fixes: facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> > index b4bfe4ddf64e..3dc62c1c2708 100644
> > --- a/fs/posix_acl.c
> > +++ b/fs/posix_acl.c
> > @@ -93,6 +93,13 @@ static void __forget_cached_acl(struct posix_acl **p)
> >  {
> >       struct posix_acl *old;
> >
> > +     /*
> > +      * ACL_DONT_CACHE is expected to be a "const" value and xchg it with
> > +      * ACL_NOT_CACHED would enable acl caching for the inode -
> > +      * clearly not what the caller has intended.
> > +      */
> > +     if (READ_ONCE(*p) == ACL_DONT_CACHE)
> > +             return;
>
> Still on vacation this week but I took a glimpse:
>
> If this isn't what the caller intended, having ACL_DONT_CACHE end up
> should be treated like a bug. So shouldn't this then be a WARN_ON_ONCE()
> and return?

The caller has requested to *forget* any *cached* *acl*.
The value ACL_DONT_CACHE means that there are no cached acls to forget
so the call is a success.
Therefore, WARN_ON is not called for IMO.

Sorry to interrupt vacation.
Let's continue when you get back.

Thanks,
Amir.

Re: [PATCH 1/2] fs: preserve ACL_DONT_CACHE state in forget_cached_acl()

[Luis Henriques]

On Tue, Jul 14 2026, Amir Goldstein wrote:

> The ACL_DONT_CACHE state is meant to be a constant state for the inode
> for filesystems that want to opt out of posix acl caching.
>
> Commit facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> used this facility to opt out of posix acl caching for fuse inodes with
> fuse server that does not negotiate FUSE_POSIX_ACL (fc->posix_acl).
>
> The commit also takes care to gate the forget_all_cached_acls() call in
> fuse_set_acl() on fc->posix_acl because there is no need for it, but
> there are other placed in fuse code which call forget_all_cached_acls()
> unconditional to fc->posix_acl and those cause the loss of the
> ACL_DONT_CACHE state.
>
> This is not only a functional bug. Properly timed, a get_acl() from this
> fuse filesystem can return a stale cached value, as was observed in
> tests, because set_acl() does not invalidate the unintentional acl
> cache.
>
> We could fix this in fuse, but it actually makes no sense for the vfs
> helper forget_cached_acl() to invalidate the ACL_DONT_CACHE state, so
> let it not do that to fix fuse and future users of ACL_DONT_CACHE.
>
> Fixes: facd61053cff1 ("fuse: fixes after adapting to new posix acl api")
> Cc: stable@vger.kernel.org Signed-off-by: Amir Goldstein
> <amir73il@gmail.com>
> ---
>  fs/posix_acl.c | 7 +++++++ 1 file changed, 7 insertions(+)
>
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c index
> b4bfe4ddf64ea..3dc62c1c27087 100644 --- a/fs/posix_acl.c +++
> b/fs/posix_acl.c @@ -93,6 +93,13 @@ static void
> __forget_cached_acl(struct posix_acl **p)
>  {
>  	struct posix_acl *old;
>  
> + /* + * ACL_DONT_CACHE is expected to be a "const" value and xchg it
> with + * ACL_NOT_CACHED would enable acl caching for the inode - + *
> clearly not what the caller has intended.  + */ + if (READ_ONCE(*p) ==
> ACL_DONT_CACHE) + return;
>  	old = xchg(p, ACL_NOT_CACHED); if (!is_uncached_acl(old))
>  		posix_acl_release(old);

Nice catch!  FWIW feel free to add my

Reviewed-by: Luis Henriques <luis@igalia.com>

Cheers,
-- 
Luís