From 94753db5ed9ad97582ef453127d9626a7a2be602 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 10 May 2012 12:19:19 -0700 Subject: [PATCH] vfs: do the careful dentry name access for all dentry_cmp cases Commit 12f8ad4b0533 ("vfs: clean up __d_lookup_rcu() and dentry_cmp() interfaces") did the careful ACCESS_ONCE() of the dentry name only for the word-at-a-time case, even though the issue is generic. Admittedly I don't really see gcc ever reloading the value in the middle of the loop, so the ACCESS_ONCE() protects us from a fairly theoretical issue. But better safe than sorry. Also, this consolidates the common parts of the word-at-a-time and bytewise logic, which includes checking the length. We'll be changing that later. Signed-off-by: Linus Torvalds --- fs/dcache.c | 54 ++++++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index c4d2ff8b4912..5c09ad7b4a15 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -153,30 +153,9 @@ int proc_nr_dentry(ctl_table *table, int write, void __user *buffer, * In contrast, 'ct' and 'tcount' can be from a pathname, and do * need the careful unaligned handling. */ -static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount) +static inline int dentry_string_cmp(const unsigned char *cs, const unsigned char *ct, unsigned tcount) { unsigned long a,b,mask; - const unsigned char *cs; - - if (unlikely(dentry->d_name.len != tcount)) - return 1; - /* - * Be careful about RCU walk racing with rename: - * use ACCESS_ONCE to fetch the name pointer. - * - * NOTE! Even if a rename will mean that the length - * was not loaded atomically, we don't care. The - * RCU walk will check the sequence count eventually, - * and catch it. And we won't overrun the buffer, - * because we're reading the name pointer atomically, - * and a dentry name is guaranteed to be properly - * terminated with a NUL byte. - * - * End result: even if 'len' is wrong, we'll exit - * early because the data cannot match (there can - * be no NUL in the ct/tcount data) - */ - cs = ACCESS_ONCE(dentry->d_name.name); for (;;) { a = *(unsigned long *)cs; @@ -197,13 +176,8 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c #else -static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount) +static inline int dentry_string_cmp(const unsigned char *cs, const unsigned char *ct, unsigned tcount) { - const unsigned char *cs = dentry->d_name.name; - - if (dentry->d_name.len != tcount) - return 1; - do { if (*cs != *ct) return 1; @@ -216,6 +190,30 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c #endif +static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *ct, unsigned tcount) +{ + if (dentry->d_name.len != tcount) + return 1; + + /* + * Be careful about RCU walk racing with rename: + * use ACCESS_ONCE to fetch the name pointer. + * + * NOTE! Even if a rename will mean that the length + * was not loaded atomically, we don't care. The + * RCU walk will check the sequence count eventually, + * and catch it. And we won't overrun the buffer, + * because we're reading the name pointer atomically, + * and a dentry name is guaranteed to be properly + * terminated with a NUL byte. + * + * End result: even if 'len' is wrong, we'll exit + * early because the data cannot match (there can + * be no NUL in the ct/tcount data) + */ + return dentry_string_cmp(ACCESS_ONCE(dentry->d_name.name), ct, tcount); +} + static void __d_free(struct rcu_head *head) { struct dentry *dentry = container_of(head, struct dentry, d_u.d_rcu); -- GitLab