vfs: be even more careful about dentry RCU name lookups
Miklos Szeredi points out that we need to also worry about memory odering when doing the dentry name comparison asynchronously with RCU. In particular, doing a rename can do a memcpy() of one dentry name over another, and we want to make sure that any unlocked reader will always see the proper terminating NUL character, so that it won't ever run off the allocation. Rather than having to be extra careful with the name copy or at lookup time for each character, this resolves the issue by making sure that all names that are inlined in the dentry always have a NUL character at the end of the name allocation. If we do that at dentry allocation time, we know that no future name copy will ever change that final NUL to anything else, so there are no memory ordering issues. So even if a concurrent rename ends up overwriting the NUL character that terminates the original name, we always know that there is one final NUL at the end, and there is no worry about the lockless RCU lookup traversing the name too far. The out-of-line allocations are never copied over, so we can just make sure that we write the name (with terminating NULL) and do a write barrier before we expose the name to anything else by setting it in the dentry. Reported-by: Miklos Szeredi <mszeredi@suse.cz> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Nick Piggin <npiggin@gmail.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
a70b52ec1a
commit
6326c71fd2
17
fs/dcache.c
17
fs/dcache.c
|
@ -192,6 +192,7 @@ static inline int dentry_string_cmp(const unsigned char *cs, const unsigned char
|
||||||
|
|
||||||
static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount)
|
static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount)
|
||||||
{
|
{
|
||||||
|
const unsigned char *cs;
|
||||||
/*
|
/*
|
||||||
* Be careful about RCU walk racing with rename:
|
* Be careful about RCU walk racing with rename:
|
||||||
* use ACCESS_ONCE to fetch the name pointer.
|
* use ACCESS_ONCE to fetch the name pointer.
|
||||||
|
@ -208,7 +209,9 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
|
||||||
* early because the data cannot match (there can
|
* early because the data cannot match (there can
|
||||||
* be no NUL in the ct/tcount data)
|
* be no NUL in the ct/tcount data)
|
||||||
*/
|
*/
|
||||||
return dentry_string_cmp(ACCESS_ONCE(dentry->d_name.name), ct, tcount);
|
cs = ACCESS_ONCE(dentry->d_name.name);
|
||||||
|
smp_read_barrier_depends();
|
||||||
|
return dentry_string_cmp(cs, ct, tcount);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void __d_free(struct rcu_head *head)
|
static void __d_free(struct rcu_head *head)
|
||||||
|
@ -1271,6 +1274,13 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
|
||||||
if (!dentry)
|
if (!dentry)
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* We guarantee that the inline name is always NUL-terminated.
|
||||||
|
* This way the memcpy() done by the name switching in rename
|
||||||
|
* will still always have a NUL at the end, even if we might
|
||||||
|
* be overwriting an internal NUL character
|
||||||
|
*/
|
||||||
|
dentry->d_iname[DNAME_INLINE_LEN-1] = 0;
|
||||||
if (name->len > DNAME_INLINE_LEN-1) {
|
if (name->len > DNAME_INLINE_LEN-1) {
|
||||||
dname = kmalloc(name->len + 1, GFP_KERNEL);
|
dname = kmalloc(name->len + 1, GFP_KERNEL);
|
||||||
if (!dname) {
|
if (!dname) {
|
||||||
|
@ -1280,13 +1290,16 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
|
||||||
} else {
|
} else {
|
||||||
dname = dentry->d_iname;
|
dname = dentry->d_iname;
|
||||||
}
|
}
|
||||||
dentry->d_name.name = dname;
|
|
||||||
|
|
||||||
dentry->d_name.len = name->len;
|
dentry->d_name.len = name->len;
|
||||||
dentry->d_name.hash = name->hash;
|
dentry->d_name.hash = name->hash;
|
||||||
memcpy(dname, name->name, name->len);
|
memcpy(dname, name->name, name->len);
|
||||||
dname[name->len] = 0;
|
dname[name->len] = 0;
|
||||||
|
|
||||||
|
/* Make sure we always see the terminating NUL character */
|
||||||
|
smp_wmb();
|
||||||
|
dentry->d_name.name = dname;
|
||||||
|
|
||||||
dentry->d_count = 1;
|
dentry->d_count = 1;
|
||||||
dentry->d_flags = 0;
|
dentry->d_flags = 0;
|
||||||
spin_lock_init(&dentry->d_lock);
|
spin_lock_init(&dentry->d_lock);
|
||||||
|
|
Loading…
Reference in New Issue