The code has been missing a way for a ->get_acl method to not cache
a return value without risking invalidating a cached value
that was set while get_acl() was returning.
Add that support by implementing to_uncachable_acl, to_cachable_acl,
is_uncacheable_acl, and dealing with uncachable acls in get_acl().
Update the comments so that they are a little clearer about
what is going on in get_acl()
Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
---
Linus my issue with the forget_cached_acl case was really that it was
too big of a hammer. If you care about caching acls only somtimes
forget_cached_acl called from ->get_acl can stomp that acl you
explicitly cached with set_cached_acl.
With this change I can unify the legacy horrible fuse posix acl case
that requires not caching acls with a single if statement in the get_acl
method. AKA:
+ if (!IS_ERR(acl) && !fc->posix_acl)
+ acl = to_uncacheable_acl(acl);
return acl;
That code I know is locally correct even if later fuse decides to cache
negative acls when the underlying filesystem does not support xattrs.
fs/posix_acl.c | 56 ++++++++++++++++++++++++++++++++++-------------
include/linux/posix_acl.h | 17 ++++++++++++++
2 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 2fd0fde16fe1..e58a68e18603 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -96,12 +96,16 @@ struct posix_acl *get_acl(struct inode *inode, int type)
{
void *sentinel;
struct posix_acl **p;
- struct posix_acl *acl;
+ struct posix_acl *acl, *to_cache;
/*
* The sentinel is used to detect when another operation like
* set_cached_acl() or forget_cached_acl() races with get_acl().
* It is guaranteed that is_uncached_acl(sentinel) is true.
+ *
+ * This is sufficient to prevent races between ->set_acl
+ * calling set_cached_acl (outside of filesystem specific
+ * locking) and get_acl() caching the returned acl.
*/
acl = get_cached_acl(inode, type);
@@ -126,12 +130,18 @@ struct posix_acl *get_acl(struct inode *inode, int type)
/* fall through */ ;
/*
- * Normally, the ACL returned by ->get_acl will be cached.
- * A filesystem can prevent that by calling
- * forget_cached_acl(inode, type) in ->get_acl.
+ * Normally, the ACL returned by ->get_acl() will be cached.
+ *
+ * A filesystem can prevent the acl returned by ->get_acl()
+ * from being cached by wrapping it with to_uncachable_acl().
+ *
+ * A filesystem can at anytime effect the cache directly and
+ * cause in process calls of get_acl() not to update the cache
+ * by calling forget_cache_acl(inode, type) or
+ * set_cached_acl(inode, type, acl).
*
- * If the filesystem doesn't have a get_acl() function at all, we'll
- * just create the negative cache entry.
+ * If the filesystem doesn't have a ->get_acl() function at
+ * all, we'll just create the negative cache entry.
*/
if (!inode->i_op->get_acl) {
set_cached_acl(inode, type, NULL);
@@ -139,21 +149,37 @@ struct posix_acl *get_acl(struct inode *inode, int type)
}
acl = inode->i_op->get_acl(inode, type);
+
+ /* To keep the logic simple default to not caching an acl when
+ * the sentinel is cleared.
+ */
+ to_cache = ACL_NOT_CACHED;
if (IS_ERR(acl)) {
- /*
- * Remove our sentinel so that we don't block future attempts
- * to cache the ACL.
+ /* Clears the sentinel so that we don't block future
+ * attempts to cache the ACL, and return an error.
*/
- cmpxchg(p, sentinel, ACL_NOT_CACHED);
- return acl;
+ }
+ else if (is_uncacheable_acl(acl)) {
+ /* Clears the sentinel so that we don't block future
+ * attempts to cache the ACL, and return a valid ACL.
+ */
+ acl = to_cacheable_acl(acl);
+ }
+ else {
+ to_cache = acl;
+ posix_acl_dup(to_cache);
}
/*
- * Cache the result, but only if our sentinel is still in place.
+ * Remove the sentinel and replace it with the value that
+ * needs to be cached, but only if the sentinel is still in
+ * place.
*/
- posix_acl_dup(acl);
- if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
- posix_acl_release(acl);
+ if (unlikely(cmpxchg(p, sentinel, to_cache) != sentinel)) {
+ if (!is_uncached_acl(to_cache))
+ posix_acl_release(to_cache);
+ }
+
return acl;
}
EXPORT_SYMBOL(get_acl);
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 540595a321a7..3be8929b9f48 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -56,6 +56,23 @@ posix_acl_release(struct posix_acl *acl)
kfree_rcu(acl, a_rcu);
}
+/*
+ * Allow for acls returned from ->get_acl() to not be cached.
+ */
+static inline bool is_uncacheable_acl(struct posix_acl *acl)
+{
+ return ((unsigned long)acl) & 1UL;
+}
+
+static inline struct posix_acl *to_uncacheable_acl(struct posix_acl *acl)
+{
+ return (struct posix_acl *)(((unsigned long)acl) | 1UL);
+}
+
+static inline struct posix_acl *to_cacheable_acl(struct posix_acl *acl)
+{
+ return (struct posix_acl *)(((unsigned long)acl) & ~1UL);
+}
/* posix_acl.c */
--
2.14.1
