Skip to content

Commit b8a7a3a

Browse files
Andreas GruenbacherAl Viro
Andreas Gruenbacher
authored and
Al Viro
committed
posix_acl: Inode acl caching fixes
When get_acl() is called for an inode whose ACL is not cached yet, the get_acl inode operation is called to fetch the ACL from the filesystem. The inode operation is responsible for updating the cached acl with set_cached_acl(). This is done without locking at the VFS level, so another task can call set_cached_acl() or forget_cached_acl() before the get_acl inode operation gets to calling set_cached_acl(), and then get_acl's call to set_cached_acl() results in caching an outdate ACL. Prevent this from happening by setting the cached ACL pointer to a task-specific sentinel value before calling the get_acl inode operation. Move the responsibility for updating the cached ACL from the get_acl inode operations to get_acl(). There, only set the cached ACL if the sentinel value hasn't changed. The sentinel values are chosen to have odd values. Likewise, the value of ACL_NOT_CACHED is odd. In contrast, ACL object pointers always have an even value (ACLs are aligned in memory). This allows to distinguish uncached ACLs values from ACL objects. In addition, switch from guarding inode->i_acl and inode->i_default_acl upates by the inode->i_lock spinlock to using xchg() and cmpxchg(). Filesystems that do not want ACLs returned from their get_acl inode operations to be cached must call forget_cached_acl() to prevent the VFS from doing so. (Patch written by Al Viro and Andreas Gruenbacher.) Signed-off-by: Andreas Gruenbacher <[email protected]> Signed-off-by: Al Viro <[email protected]>
1 parent 8861964 commit b8a7a3a

File tree

17 files changed

+138
-78
lines changed

17 files changed

+138
-78
lines changed

‎fs/9p/acl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ static struct posix_acl *v9fs_get_cached_acl(struct inode *inode, int type)
9393
* instantiating the inode (v9fs_inode_from_fid)
9494
*/
9595
acl = get_cached_acl(inode, type);
96-
BUG_ON(acl == ACL_NOT_CACHED);
96+
BUG_ON(is_uncached_acl(acl));
9797
return acl;
9898
}
9999

‎fs/btrfs/acl.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
6363
}
6464
kfree(value);
6565

66-
if (!IS_ERR(acl))
67-
set_cached_acl(inode, type, acl);
68-
6966
return acl;
7067
}
7168

‎fs/ceph/acl.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ static inline void ceph_set_cached_acl(struct inode *inode,
3737
spin_lock(&ci->i_ceph_lock);
3838
if (__ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 0))
3939
set_cached_acl(inode, type, acl);
40+
else
41+
forget_cached_acl(inode, type);
4042
spin_unlock(&ci->i_ceph_lock);
4143
}
4244

‎fs/ext2/acl.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,6 @@ ext2_get_acl(struct inode *inode, int type)
172172
acl = ERR_PTR(retval);
173173
kfree(value);
174174

175-
if (!IS_ERR(acl))
176-
set_cached_acl(inode, type, acl);
177-
178175
return acl;
179176
}
180177

‎fs/ext4/acl.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,6 @@ ext4_get_acl(struct inode *inode, int type)
172172
acl = ERR_PTR(retval);
173173
kfree(value);
174174

175-
if (!IS_ERR(acl))
176-
set_cached_acl(inode, type, acl);
177-
178175
return acl;
179176
}
180177

‎fs/f2fs/acl.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,6 @@ static struct posix_acl *__f2fs_get_acl(struct inode *inode, int type,
190190
acl = ERR_PTR(retval);
191191
kfree(value);
192192

193-
if (!IS_ERR(acl))
194-
set_cached_acl(inode, type, acl);
195-
196193
return acl;
197194
}
198195

‎fs/hfsplus/posix_acl.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ struct posix_acl *hfsplus_get_posix_acl(struct inode *inode, int type)
4848

4949
hfsplus_destroy_attr_entry((hfsplus_attr_entry *)value);
5050

51-
if (!IS_ERR(acl))
52-
set_cached_acl(inode, type, acl);
53-
5451
return acl;
5552
}
5653

‎fs/inode.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,9 @@ void __destroy_inode(struct inode *inode)
238238
}
239239

240240
#ifdef CONFIG_FS_POSIX_ACL
241-
if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
241+
if (inode->i_acl && !is_uncached_acl(inode->i_acl))
242242
posix_acl_release(inode->i_acl);
243-
if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
243+
if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl))
244244
posix_acl_release(inode->i_default_acl);
245245
#endif
246246
this_cpu_dec(nr_inodes);

‎fs/jffs2/acl.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,6 @@ struct posix_acl *jffs2_get_acl(struct inode *inode, int type)
203203
acl = ERR_PTR(rc);
204204
}
205205
kfree(value);
206-
if (!IS_ERR(acl))
207-
set_cached_acl(inode, type, acl);
208206
return acl;
209207
}
210208

‎fs/jfs/acl.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ struct posix_acl *jfs_get_acl(struct inode *inode, int type)
6363
acl = posix_acl_from_xattr(&init_user_ns, value, size);
6464
}
6565
kfree(value);
66-
if (!IS_ERR(acl))
67-
set_cached_acl(inode, type, acl);
6866
return acl;
6967
}
7068

‎fs/namei.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ static int check_acl(struct inode *inode, int mask)
265265
if (!acl)
266266
return -EAGAIN;
267267
/* no ->get_acl() calls in RCU mode... */
268-
if (acl == ACL_NOT_CACHED)
268+
if (is_uncached_acl(acl))
269269
return -ECHILD;
270270
return posix_acl_permission(inode, acl, mask & ~MAY_NOT_BLOCK);
271271
}

‎fs/nfs/nfs3acl.c

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,38 @@
1111

1212
#define NFSDBG_FACILITY NFSDBG_PROC
1313

14+
/*
15+
* nfs3_prepare_get_acl, nfs3_complete_get_acl, nfs3_abort_get_acl: Helpers for
16+
* caching get_acl results in a race-free way. See fs/posix_acl.c:get_acl()
17+
* for explanations.
18+
*/
19+
static void nfs3_prepare_get_acl(struct posix_acl **p)
20+
{
21+
struct posix_acl *sentinel = uncached_acl_sentinel(current);
22+
23+
if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) {
24+
/* Not the first reader or sentinel already in place. */
25+
}
26+
}
27+
28+
static void nfs3_complete_get_acl(struct posix_acl **p, struct posix_acl *acl)
29+
{
30+
struct posix_acl *sentinel = uncached_acl_sentinel(current);
31+
32+
/* Only cache the ACL if our sentinel is still in place. */
33+
posix_acl_dup(acl);
34+
if (cmpxchg(p, sentinel, acl) != sentinel)
35+
posix_acl_release(acl);
36+
}
37+
38+
static void nfs3_abort_get_acl(struct posix_acl **p)
39+
{
40+
struct posix_acl *sentinel = uncached_acl_sentinel(current);
41+
42+
/* Remove our sentinel upon failure. */
43+
cmpxchg(p, sentinel, ACL_NOT_CACHED);
44+
}
45+
1446
struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
1547
{
1648
struct nfs_server *server = NFS_SERVER(inode);
@@ -55,6 +87,11 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
5587
if (res.fattr == NULL)
5688
return ERR_PTR(-ENOMEM);
5789

90+
if (args.mask & NFS_ACL)
91+
nfs3_prepare_get_acl(&inode->i_acl);
92+
if (args.mask & NFS_DFACL)
93+
nfs3_prepare_get_acl(&inode->i_default_acl);
94+
5895
status = rpc_call_sync(server->client_acl, &msg, 0);
5996
dprintk("NFS reply getacl: %d\n", status);
6097

@@ -89,12 +126,12 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
89126
}
90127

91128
if (res.mask & NFS_ACL)
92-
set_cached_acl(inode, ACL_TYPE_ACCESS, res.acl_access);
129+
nfs3_complete_get_acl(&inode->i_acl, res.acl_access);
93130
else
94131
forget_cached_acl(inode, ACL_TYPE_ACCESS);
95132

96133
if (res.mask & NFS_DFACL)
97-
set_cached_acl(inode, ACL_TYPE_DEFAULT, res.acl_default);
134+
nfs3_complete_get_acl(&inode->i_default_acl, res.acl_default);
98135
else
99136
forget_cached_acl(inode, ACL_TYPE_DEFAULT);
100137

@@ -108,6 +145,8 @@ struct posix_acl *nfs3_get_acl(struct inode *inode, int type)
108145
}
109146

110147
getout:
148+
nfs3_abort_get_acl(&inode->i_acl);
149+
nfs3_abort_get_acl(&inode->i_default_acl);
111150
posix_acl_release(res.acl_access);
112151
posix_acl_release(res.acl_default);
113152
nfs_free_fattr(res.fattr);

‎fs/ocfs2/dlmglue.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include "uptodate.h"
5555
#include "quota.h"
5656
#include "refcounttree.h"
57+
#include "acl.h"
5758

5859
#include "buffer_head_io.h"
5960

@@ -3623,6 +3624,8 @@ static int ocfs2_data_convert_worker(struct ocfs2_lock_res *lockres,
36233624
filemap_fdatawait(mapping);
36243625
}
36253626

3627+
forget_all_cached_acls(inode);
3628+
36263629
out:
36273630
return UNBLOCK_CONTINUE;
36283631
}

‎fs/posix_acl.c

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,18 @@ EXPORT_SYMBOL(acl_by_type);
3737
struct posix_acl *get_cached_acl(struct inode *inode, int type)
3838
{
3939
struct posix_acl **p = acl_by_type(inode, type);
40-
struct posix_acl *acl = ACCESS_ONCE(*p);
41-
if (acl) {
42-
spin_lock(&inode->i_lock);
43-
acl = *p;
44-
if (acl != ACL_NOT_CACHED)
45-
acl = posix_acl_dup(acl);
46-
spin_unlock(&inode->i_lock);
40+
struct posix_acl *acl;
41+
42+
for (;;) {
43+
rcu_read_lock();
44+
acl = rcu_dereference(*p);
45+
if (!acl || is_uncached_acl(acl) ||
46+
atomic_inc_not_zero(&acl->a_refcount))
47+
break;
48+
rcu_read_unlock();
49+
cpu_relax();
4750
}
51+
rcu_read_unlock();
4852
return acl;
4953
}
5054
EXPORT_SYMBOL(get_cached_acl);
@@ -59,58 +63,72 @@ void set_cached_acl(struct inode *inode, int type, struct posix_acl *acl)
5963
{
6064
struct posix_acl **p = acl_by_type(inode, type);
6165
struct posix_acl *old;
62-
spin_lock(&inode->i_lock);
63-
old = *p;
64-
rcu_assign_pointer(*p, posix_acl_dup(acl));
65-
spin_unlock(&inode->i_lock);
66-
if (old != ACL_NOT_CACHED)
66+
67+
old = xchg(p, posix_acl_dup(acl));
68+
if (!is_uncached_acl(old))
6769
posix_acl_release(old);
6870
}
6971
EXPORT_SYMBOL(set_cached_acl);
7072

71-
void forget_cached_acl(struct inode *inode, int type)
73+
static void __forget_cached_acl(struct posix_acl **p)
7274
{
73-
struct posix_acl **p = acl_by_type(inode, type);
7475
struct posix_acl *old;
75-
spin_lock(&inode->i_lock);
76-
old = *p;
77-
*p = ACL_NOT_CACHED;
78-
spin_unlock(&inode->i_lock);
79-
if (old != ACL_NOT_CACHED)
76+
77+
old = xchg(p, ACL_NOT_CACHED);
78+
if (!is_uncached_acl(old))
8079
posix_acl_release(old);
8180
}
81+
82+
void forget_cached_acl(struct inode *inode, int type)
83+
{
84+
__forget_cached_acl(acl_by_type(inode, type));
85+
}
8286
EXPORT_SYMBOL(forget_cached_acl);
8387

8488
void forget_all_cached_acls(struct inode *inode)
8589
{
86-
struct posix_acl *old_access, *old_default;
87-
spin_lock(&inode->i_lock);
88-
old_access = inode->i_acl;
89-
old_default = inode->i_default_acl;
90-
inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
91-
spin_unlock(&inode->i_lock);
92-
if (old_access != ACL_NOT_CACHED)
93-
posix_acl_release(old_access);
94-
if (old_default != ACL_NOT_CACHED)
95-
posix_acl_release(old_default);
90+
__forget_cached_acl(&inode->i_acl);
91+
__forget_cached_acl(&inode->i_default_acl);
9692
}
9793
EXPORT_SYMBOL(forget_all_cached_acls);
9894

9995
struct posix_acl *get_acl(struct inode *inode, int type)
10096
{
97+
void *sentinel;
98+
struct posix_acl **p;
10199
struct posix_acl *acl;
102100

101+
/*
102+
* The sentinel is used to detect when another operation like
103+
* set_cached_acl() or forget_cached_acl() races with get_acl().
104+
* It is guaranteed that is_uncached_acl(sentinel) is true.
105+
*/
106+
103107
acl = get_cached_acl(inode, type);
104-
if (acl != ACL_NOT_CACHED)
108+
if (!is_uncached_acl(acl))
105109
return acl;
106110

107111
if (!IS_POSIXACL(inode))
108112
return NULL;
109113

114+
sentinel = uncached_acl_sentinel(current);
115+
p = acl_by_type(inode, type);
116+
110117
/*
111-
* A filesystem can force a ACL callback by just never filling the
112-
* ACL cache. But normally you'd fill the cache either at inode
113-
* instantiation time, or on the first ->get_acl call.
118+
* If the ACL isn't being read yet, set our sentinel. Otherwise, the
119+
* current value of the ACL will not be ACL_NOT_CACHED and so our own
120+
* sentinel will not be set; another task will update the cache. We
121+
* could wait for that other task to complete its job, but it's easier
122+
* to just call ->get_acl to fetch the ACL ourself. (This is going to
123+
* be an unlikely race.)
124+
*/
125+
if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED)
126+
/* fall through */ ;
127+
128+
/*
129+
* Normally, the ACL returned by ->get_acl will be cached.
130+
* A filesystem can prevent that by calling
131+
* forget_cached_acl(inode, type) in ->get_acl.
114132
*
115133
* If the filesystem doesn't have a get_acl() function at all, we'll
116134
* just create the negative cache entry.
@@ -119,7 +137,24 @@ struct posix_acl *get_acl(struct inode *inode, int type)
119137
set_cached_acl(inode, type, NULL);
120138
return NULL;
121139
}
122-
return inode->i_op->get_acl(inode, type);
140+
acl = inode->i_op->get_acl(inode, type);
141+
142+
if (IS_ERR(acl)) {
143+
/*
144+
* Remove our sentinel so that we don't block future attempts
145+
* to cache the ACL.
146+
*/
147+
cmpxchg(p, sentinel, ACL_NOT_CACHED);
148+
return acl;
149+
}
150+
151+
/*
152+
* Cache the result, but only if our sentinel is still in place.
153+
*/
154+
posix_acl_dup(acl);
155+
if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
156+
posix_acl_release(acl);
157+
return acl;
123158
}
124159
EXPORT_SYMBOL(get_acl);
125160

‎fs/reiserfs/xattr_acl.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,8 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type)
197197

198198
size = reiserfs_xattr_get(inode, name, NULL, 0);
199199
if (size < 0) {
200-
if (size == -ENODATA || size == -ENOSYS) {
201-
set_cached_acl(inode, type, NULL);
200+
if (size == -ENODATA || size == -ENOSYS)
202201
return NULL;
203-
}
204202
return ERR_PTR(size);
205203
}
206204

@@ -220,8 +218,6 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type)
220218
} else {
221219
acl = reiserfs_posix_acl_from_disk(value, retval);
222220
}
223-
if (!IS_ERR(acl))
224-
set_cached_acl(inode, type, acl);
225221

226222
kfree(value);
227223
return acl;

‎fs/xfs/xfs_acl.c

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,14 @@ xfs_get_acl(struct inode *inode, int type)
158158
if (error) {
159159
/*
160160
* If the attribute doesn't exist make sure we have a negative
161-
* cache entry, for any other error assume it is transient and
162-
* leave the cache entry as ACL_NOT_CACHED.
161+
* cache entry, for any other error assume it is transient.
163162
*/
164-
if (error == -ENOATTR)
165-
goto out_update_cache;
166-
acl = ERR_PTR(error);
167-
goto out;
163+
if (error != -ENOATTR)
164+
acl = ERR_PTR(error);
165+
} else {
166+
acl = xfs_acl_from_disk(xfs_acl, len,
167+
XFS_ACL_MAX_ENTRIES(ip->i_mount));
168168
}
169-
170-
acl = xfs_acl_from_disk(xfs_acl, len, XFS_ACL_MAX_ENTRIES(ip->i_mount));
171-
if (IS_ERR(acl))
172-
goto out;
173-
174-
out_update_cache:
175-
set_cached_acl(inode, type, acl);
176-
out:
177169
kmem_free(xfs_acl);
178170
return acl;
179171
}

0 commit comments

Comments
 (0)