Skip to content

Commit

Permalink
Don't pass inode to ->d_hash() and ->d_compare()
Browse files Browse the repository at this point in the history
Instances either don't look at it at all (the majority of cases) or
only want it to find the superblock (which can be had as dentry->d_sb).
A few cases that want more are actually safe with dentry->d_inode -
the only precaution needed is the check that it hadn't been replaced with
NULL by rmdir() or by overwriting rename(), which case should be simply
treated as cache miss.

Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Al Viro <[email protected]>
  • Loading branch information
torvalds authored and Al Viro committed Jun 29, 2013
1 parent 642b704 commit da53be1
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 165 deletions.
6 changes: 2 additions & 4 deletions Documentation/filesystems/Locking
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,8 @@ be able to use diff(1).
prototypes:
int (*d_revalidate)(struct dentry *, unsigned int);
int (*d_weak_revalidate)(struct dentry *, unsigned int);
int (*d_hash)(const struct dentry *, const struct inode *,
struct qstr *);
int (*d_compare)(const struct dentry *, const struct inode *,
const struct dentry *, const struct inode *,
int (*d_hash)(const struct dentry *, struct qstr *);
int (*d_compare)(const struct dentry *, const struct dentry *,
unsigned int, const char *, const struct qstr *);
int (*d_delete)(struct dentry *);
void (*d_release)(struct dentry *);
Expand Down
19 changes: 8 additions & 11 deletions Documentation/filesystems/vfs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -901,10 +901,8 @@ defined:
struct dentry_operations {
int (*d_revalidate)(struct dentry *, unsigned int);
int (*d_weak_revalidate)(struct dentry *, unsigned int);
int (*d_hash)(const struct dentry *, const struct inode *,
struct qstr *);
int (*d_compare)(const struct dentry *, const struct inode *,
const struct dentry *, const struct inode *,
int (*d_hash)(const struct dentry *, struct qstr *);
int (*d_compare)(const struct dentry *, const struct dentry *,
unsigned int, const char *, const struct qstr *);
int (*d_delete)(const struct dentry *);
void (*d_release)(struct dentry *);
Expand Down Expand Up @@ -949,25 +947,24 @@ struct dentry_operations {

d_hash: called when the VFS adds a dentry to the hash table. The first
dentry passed to d_hash is the parent directory that the name is
to be hashed into. The inode is the dentry's inode.
to be hashed into.

Same locking and synchronisation rules as d_compare regarding
what is safe to dereference etc.

d_compare: called to compare a dentry name with a given name. The first
dentry is the parent of the dentry to be compared, the second is
the parent's inode, then the dentry and inode (may be NULL) of the
child dentry. len and name string are properties of the dentry to be
compared. qstr is the name to compare it with.
the child dentry. len and name string are properties of the dentry
to be compared. qstr is the name to compare it with.

Must be constant and idempotent, and should not take locks if
possible, and should not or store into the dentry or inodes.
Should not dereference pointers outside the dentry or inodes without
possible, and should not or store into the dentry.
Should not dereference pointers outside the dentry without
lots of care (eg. d_parent, d_inode, d_name should not be used).

However, our vfsmount is pinned, and RCU held, so the dentries and
inodes won't disappear, neither will our sb or filesystem module.
->i_sb and ->d_sb may be used.
->d_sb may be used.

It is a tricky calling convention because it needs to be called under
"rcu-walk", ie. without any locks or references on things.
Expand Down
6 changes: 2 additions & 4 deletions fs/adfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,7 @@ const struct file_operations adfs_dir_operations = {
};

static int
adfs_hash(const struct dentry *parent, const struct inode *inode,
struct qstr *qstr)
adfs_hash(const struct dentry *parent, struct qstr *qstr)
{
const unsigned int name_len = ADFS_SB(parent->d_sb)->s_namelen;
const unsigned char *name;
Expand Down Expand Up @@ -228,8 +227,7 @@ adfs_hash(const struct dentry *parent, const struct inode *inode,
* requirements of the underlying filesystem.
*/
static int
adfs_compare(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
adfs_compare(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
int i;
Expand Down
26 changes: 8 additions & 18 deletions fs/affs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,12 @@
typedef int (*toupper_t)(int);

static int affs_toupper(int ch);
static int affs_hash_dentry(const struct dentry *,
const struct inode *, struct qstr *);
static int affs_compare_dentry(const struct dentry *parent,
const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int affs_hash_dentry(const struct dentry *, struct qstr *);
static int affs_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name);
static int affs_intl_toupper(int ch);
static int affs_intl_hash_dentry(const struct dentry *,
const struct inode *, struct qstr *);
static int affs_intl_compare_dentry(const struct dentry *parent,
const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int affs_intl_hash_dentry(const struct dentry *, struct qstr *);
static int affs_intl_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name);

const struct dentry_operations affs_dentry_operations = {
Expand Down Expand Up @@ -86,14 +80,12 @@ __affs_hash_dentry(struct qstr *qstr, toupper_t toupper)
}

static int
affs_hash_dentry(const struct dentry *dentry, const struct inode *inode,
struct qstr *qstr)
affs_hash_dentry(const struct dentry *dentry, struct qstr *qstr)
{
return __affs_hash_dentry(qstr, affs_toupper);
}
static int
affs_intl_hash_dentry(const struct dentry *dentry, const struct inode *inode,
struct qstr *qstr)
affs_intl_hash_dentry(const struct dentry *dentry, struct qstr *qstr)
{
return __affs_hash_dentry(qstr, affs_intl_toupper);
}
Expand Down Expand Up @@ -131,15 +123,13 @@ static inline int __affs_compare_dentry(unsigned int len,
}

static int
affs_compare_dentry(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
affs_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
return __affs_compare_dentry(len, str, name, affs_toupper);
}
static int
affs_intl_compare_dentry(const struct dentry *parent,const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
affs_intl_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
return __affs_compare_dentry(len, str, name, affs_intl_toupper);
Expand Down
9 changes: 3 additions & 6 deletions fs/cifs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,7 @@ const struct dentry_operations cifs_dentry_ops = {
/* d_delete: cifs_d_delete, */ /* not needed except for debugging */
};

static int cifs_ci_hash(const struct dentry *dentry, const struct inode *inode,
struct qstr *q)
static int cifs_ci_hash(const struct dentry *dentry, struct qstr *q)
{
struct nls_table *codepage = CIFS_SB(dentry->d_sb)->local_nls;
unsigned long hash;
Expand All @@ -838,12 +837,10 @@ static int cifs_ci_hash(const struct dentry *dentry, const struct inode *inode,
return 0;
}

static int cifs_ci_compare(const struct dentry *parent,
const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int cifs_ci_compare(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
struct nls_table *codepage = CIFS_SB(pinode->i_sb)->local_nls;
struct nls_table *codepage = CIFS_SB(parent->d_sb)->local_nls;

if ((name->len == len) &&
(nls_strnicmp(codepage, name->name, str, len) == 0))
Expand Down
27 changes: 10 additions & 17 deletions fs/dcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1723,7 +1723,7 @@ EXPORT_SYMBOL(d_add_ci);
* Do the slow-case of the dentry name compare.
*
* Unlike the dentry_cmp() function, we need to atomically
* load the name, length and inode information, so that the
* load the name and length information, so that the
* filesystem can rely on them, and can use the 'name' and
* 'len' information without worrying about walking off the
* end of memory etc.
Expand All @@ -1741,22 +1741,18 @@ enum slow_d_compare {

static noinline enum slow_d_compare slow_dentry_cmp(
const struct dentry *parent,
struct inode *inode,
struct dentry *dentry,
unsigned int seq,
const struct qstr *name)
{
int tlen = dentry->d_name.len;
const char *tname = dentry->d_name.name;
struct inode *i = dentry->d_inode;

if (read_seqcount_retry(&dentry->d_seq, seq)) {
cpu_relax();
return D_COMP_SEQRETRY;
}
if (parent->d_op->d_compare(parent, inode,
dentry, i,
tlen, tname, name))
if (parent->d_op->d_compare(parent, dentry, tlen, tname, name))
return D_COMP_NOMATCH;
return D_COMP_OK;
}
Expand All @@ -1766,7 +1762,6 @@ static noinline enum slow_d_compare slow_dentry_cmp(
* @parent: parent dentry
* @name: qstr of name we wish to find
* @seqp: returns d_seq value at the point where the dentry was found
* @inode: returns dentry->d_inode when the inode was found valid.
* Returns: dentry, or NULL
*
* __d_lookup_rcu is the dcache lookup function for rcu-walk name
Expand All @@ -1793,7 +1788,7 @@ static noinline enum slow_d_compare slow_dentry_cmp(
*/
struct dentry *__d_lookup_rcu(const struct dentry *parent,
const struct qstr *name,
unsigned *seqp, struct inode *inode)
unsigned *seqp)
{
u64 hashlen = name->hash_len;
const unsigned char *str = name->name;
Expand Down Expand Up @@ -1827,11 +1822,10 @@ struct dentry *__d_lookup_rcu(const struct dentry *parent,
seqretry:
/*
* The dentry sequence count protects us from concurrent
* renames, and thus protects inode, parent and name fields.
* renames, and thus protects parent and name fields.
*
* The caller must perform a seqcount check in order
* to do anything useful with the returned dentry,
* including using the 'd_inode' pointer.
* to do anything useful with the returned dentry.
*
* NOTE! We do a "raw" seqcount_begin here. That means that
* we don't wait for the sequence count to stabilize if it
Expand All @@ -1845,12 +1839,12 @@ struct dentry *__d_lookup_rcu(const struct dentry *parent,
continue;
if (d_unhashed(dentry))
continue;
*seqp = seq;

if (unlikely(parent->d_flags & DCACHE_OP_COMPARE)) {
if (dentry->d_name.hash != hashlen_hash(hashlen))
continue;
switch (slow_dentry_cmp(parent, inode, dentry, seq, name)) {
*seqp = seq;
switch (slow_dentry_cmp(parent, dentry, seq, name)) {
case D_COMP_OK:
return dentry;
case D_COMP_NOMATCH:
Expand All @@ -1862,6 +1856,7 @@ struct dentry *__d_lookup_rcu(const struct dentry *parent,

if (dentry->d_name.hash_len != hashlen)
continue;
*seqp = seq;
if (!dentry_cmp(dentry, str, hashlen_len(hashlen)))
return dentry;
}
Expand Down Expand Up @@ -1959,9 +1954,7 @@ struct dentry *__d_lookup(const struct dentry *parent, const struct qstr *name)
if (parent->d_flags & DCACHE_OP_COMPARE) {
int tlen = dentry->d_name.len;
const char *tname = dentry->d_name.name;
if (parent->d_op->d_compare(parent, parent->d_inode,
dentry, dentry->d_inode,
tlen, tname, name))
if (parent->d_op->d_compare(parent, dentry, tlen, tname, name))
goto next;
} else {
if (dentry->d_name.len != len)
Expand Down Expand Up @@ -1998,7 +1991,7 @@ struct dentry *d_hash_and_lookup(struct dentry *dir, struct qstr *name)
*/
name->hash = full_name_hash(name->name, name->len);
if (dir->d_flags & DCACHE_OP_HASH) {
int err = dir->d_op->d_hash(dir, dir->d_inode, name);
int err = dir->d_op->d_hash(dir, name);
if (unlikely(err < 0))
return ERR_PTR(err);
}
Expand Down
9 changes: 4 additions & 5 deletions fs/efivarfs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ static struct super_block *efivarfs_sb;
* So we need to perform a case-sensitive match on part 1 and a
* case-insensitive match on part 2.
*/
static int efivarfs_d_compare(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int efivarfs_d_compare(const struct dentry *parent,
const struct dentry *dentry,
unsigned int len, const char *str,
const struct qstr *name)
{
Expand All @@ -63,8 +63,7 @@ static int efivarfs_d_compare(const struct dentry *parent, const struct inode *p
return strncasecmp(name->name + guid, str + guid, EFI_VARIABLE_GUID_LEN);
}

static int efivarfs_d_hash(const struct dentry *dentry,
const struct inode *inode, struct qstr *qstr)
static int efivarfs_d_hash(const struct dentry *dentry, struct qstr *qstr)
{
unsigned long hash = init_name_hash();
const unsigned char *s = qstr->name;
Expand Down Expand Up @@ -108,7 +107,7 @@ static struct dentry *efivarfs_alloc_dentry(struct dentry *parent, char *name)
q.name = name;
q.len = strlen(name);

err = efivarfs_d_hash(NULL, NULL, &q);
err = efivarfs_d_hash(NULL, &q);
if (err)
return ERR_PTR(err);

Expand Down
6 changes: 2 additions & 4 deletions fs/fat/namei_msdos.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ static int msdos_find(struct inode *dir, const unsigned char *name, int len,
* that the existing dentry can be used. The msdos fs routines will
* return ENOENT or EINVAL as appropriate.
*/
static int msdos_hash(const struct dentry *dentry, const struct inode *inode,
struct qstr *qstr)
static int msdos_hash(const struct dentry *dentry, struct qstr *qstr)
{
struct fat_mount_options *options = &MSDOS_SB(dentry->d_sb)->options;
unsigned char msdos_name[MSDOS_NAME];
Expand All @@ -165,8 +164,7 @@ static int msdos_hash(const struct dentry *dentry, const struct inode *inode,
* Compare two msdos names. If either of the names are invalid,
* we fall back to doing the standard name comparison.
*/
static int msdos_cmp(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int msdos_cmp(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
struct fat_mount_options *options = &MSDOS_SB(parent->d_sb)->options;
Expand Down
12 changes: 4 additions & 8 deletions fs/fat/namei_vfat.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ static unsigned int vfat_striptail_len(const struct qstr *qstr)
* that the existing dentry can be used. The vfat fs routines will
* return ENOENT or EINVAL as appropriate.
*/
static int vfat_hash(const struct dentry *dentry, const struct inode *inode,
struct qstr *qstr)
static int vfat_hash(const struct dentry *dentry, struct qstr *qstr)
{
qstr->hash = full_name_hash(qstr->name, vfat_striptail_len(qstr));
return 0;
Expand All @@ -120,8 +119,7 @@ static int vfat_hash(const struct dentry *dentry, const struct inode *inode,
* that the existing dentry can be used. The vfat fs routines will
* return ENOENT or EINVAL as appropriate.
*/
static int vfat_hashi(const struct dentry *dentry, const struct inode *inode,
struct qstr *qstr)
static int vfat_hashi(const struct dentry *dentry, struct qstr *qstr)
{
struct nls_table *t = MSDOS_SB(dentry->d_sb)->nls_io;
const unsigned char *name;
Expand All @@ -142,8 +140,7 @@ static int vfat_hashi(const struct dentry *dentry, const struct inode *inode,
/*
* Case insensitive compare of two vfat names.
*/
static int vfat_cmpi(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int vfat_cmpi(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
struct nls_table *t = MSDOS_SB(parent->d_sb)->nls_io;
Expand All @@ -162,8 +159,7 @@ static int vfat_cmpi(const struct dentry *parent, const struct inode *pinode,
/*
* Case sensitive compare of two vfat names.
*/
static int vfat_cmp(const struct dentry *parent, const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
static int vfat_cmp(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name)
{
unsigned int alen, blen;
Expand Down
3 changes: 1 addition & 2 deletions fs/gfs2/dentry.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
return 0;
}

static int gfs2_dhash(const struct dentry *dentry, const struct inode *inode,
struct qstr *str)
static int gfs2_dhash(const struct dentry *dentry, struct qstr *str)
{
str->hash = gfs2_disk_hash(str->name, str->len);
return 0;
Expand Down
7 changes: 2 additions & 5 deletions fs/hfs/hfs_fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,10 @@ extern int hfs_part_find(struct super_block *, sector_t *, sector_t *);
/* string.c */
extern const struct dentry_operations hfs_dentry_operations;

extern int hfs_hash_dentry(const struct dentry *, const struct inode *,
struct qstr *);
extern int hfs_hash_dentry(const struct dentry *, struct qstr *);
extern int hfs_strcmp(const unsigned char *, unsigned int,
const unsigned char *, unsigned int);
extern int hfs_compare_dentry(const struct dentry *parent,
const struct inode *pinode,
const struct dentry *dentry, const struct inode *inode,
extern int hfs_compare_dentry(const struct dentry *parent, const struct dentry *dentry,
unsigned int len, const char *str, const struct qstr *name);

/* trans.c */
Expand Down
Loading

0 comments on commit da53be1

Please sign in to comment.