Skip to content

Commit

Permalink
Merge tag 'pull-revalidate' of git://git.kernel.org/pub/scm/linux/ker…
Browse files Browse the repository at this point in the history
…nel/git/viro/vfs

Pull vfs d_revalidate updates from Al Viro:
 "Provide stable parent and name to ->d_revalidate() instances

  Most of the filesystem methods where we care about dentry name and
  parent have their stability guaranteed by the callers;
  ->d_revalidate() is the major exception.

  It's easy enough for callers to supply stable values for expected name
  and expected parent of the dentry being validated. That kills quite a
  bit of boilerplate in ->d_revalidate() instances, along with a bunch
  of races where they used to access ->d_name without sufficient
  precautions"

* tag 'pull-revalidate' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  9p: fix ->rename_sem exclusion
  orangefs_d_revalidate(): use stable parent inode and name passed by caller
  ocfs2_dentry_revalidate(): use stable parent inode and name passed by caller
  nfs: fix ->d_revalidate() UAF on ->d_name accesses
  nfs{,4}_lookup_validate(): use stable parent inode passed by caller
  gfs2_drevalidate(): use stable parent inode and name passed by caller
  fuse_dentry_revalidate(): use stable parent inode and name passed by caller
  vfat_revalidate{,_ci}(): use stable parent inode passed by caller
  exfat_d_revalidate(): use stable parent inode passed by caller
  fscrypt_d_revalidate(): use stable parent inode passed by caller
  ceph_d_revalidate(): propagate stable name down into request encoding
  ceph_d_revalidate(): use stable parent inode passed by caller
  afs_d_revalidate(): use stable name and parent inode passed by caller
  Pass parent directory inode and expected name to ->d_revalidate()
  generic_ci_d_compare(): use shortname_storage
  ext4 fast_commit: make use of name_snapshot primitives
  dissolve external_name.u into separate members
  make take_dentry_name_snapshot() lockless
  dcache: back inline names with a struct-wrapped array of unsigned long
  make sure that DNAME_INLINE_LEN is a multiple of word size
  • Loading branch information
Linus Torvalds committed Jan 30, 2025
2 parents ce33580 + 30d61ef commit d3d90cc
Show file tree
Hide file tree
Showing 43 changed files with 359 additions and 307 deletions.
7 changes: 6 additions & 1 deletion Documentation/filesystems/locking.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ dentry_operations

prototypes::

int (*d_revalidate)(struct dentry *, unsigned int);
int (*d_revalidate)(struct inode *, const struct qstr *,
struct dentry *, unsigned int);
int (*d_weak_revalidate)(struct dentry *, unsigned int);
int (*d_hash)(const struct dentry *, struct qstr *);
int (*d_compare)(const struct dentry *,
Expand All @@ -30,6 +31,8 @@ prototypes::
struct vfsmount *(*d_automount)(struct path *path);
int (*d_manage)(const struct path *, bool);
struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
bool (*d_unalias_trylock)(const struct dentry *);
void (*d_unalias_unlock)(const struct dentry *);

locking rules:

Expand All @@ -49,6 +52,8 @@ d_dname: no no no no
d_automount: no no yes no
d_manage: no no yes (ref-walk) maybe
d_real no no yes no
d_unalias_trylock yes no no no
d_unalias_unlock yes no no no
================== =========== ======== ============== ========

inode_operations
Expand Down
16 changes: 16 additions & 0 deletions Documentation/filesystems/porting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1141,3 +1141,19 @@ pointer are gone.

set_blocksize() takes opened struct file instead of struct block_device now
and it *must* be opened exclusive.

---

** mandatory**

->d_revalidate() gets two extra arguments - inode of parent directory and
name our dentry is expected to have. Both are stable (dir is pinned in
non-RCU case and will stay around during the call in RCU case, and name
is guaranteed to stay unchanging). Your instance doesn't have to use
either, but it often helps to avoid a lot of painful boilerplate.
Note that while name->name is stable and NUL-terminated, it may (and
often will) have name->name[name->len] equal to '/' rather than '\0' -
in normal case it points into the pathname being looked up.
NOTE: if you need something like full path from the root of filesystem,
you are still on your own - this assists with simple cases, but it's not
magic.
24 changes: 23 additions & 1 deletion Documentation/filesystems/vfs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,8 @@ defined:
.. code-block:: c
struct dentry_operations {
int (*d_revalidate)(struct dentry *, unsigned int);
int (*d_revalidate)(struct inode *, const struct qstr *,
struct dentry *, unsigned int);
int (*d_weak_revalidate)(struct dentry *, unsigned int);
int (*d_hash)(const struct dentry *, struct qstr *);
int (*d_compare)(const struct dentry *,
Expand All @@ -1264,6 +1265,8 @@ defined:
struct vfsmount *(*d_automount)(struct path *);
int (*d_manage)(const struct path *, bool);
struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
bool (*d_unalias_trylock)(const struct dentry *);
void (*d_unalias_unlock)(const struct dentry *);
};
``d_revalidate``
Expand Down Expand Up @@ -1427,6 +1430,25 @@ defined:

For non-regular files, the 'dentry' argument is returned.

``d_unalias_trylock``
if present, will be called by d_splice_alias() before moving a
preexisting attached alias. Returning false prevents __d_move(),
making d_splice_alias() fail with -ESTALE.

Rationale: setting FS_RENAME_DOES_D_MOVE will prevent d_move()
and d_exchange() calls from the outside of filesystem methods;
however, it does not guarantee that attached dentries won't
be renamed or moved by d_splice_alias() finding a preexisting
alias for a directory inode. Normally we would not care;
however, something that wants to stabilize the entire path to
root over a blocking operation might need that. See 9p for one
(and hopefully only) example.

``d_unalias_unlock``
should be paired with ``d_unalias_trylock``; that one is called after
__d_move() call in __d_unalias().


Each dentry has a pointer to its parent dentry, as well as a hash list
of child dentries. Child dentries are basically like files in a
directory.
Expand Down
2 changes: 1 addition & 1 deletion fs/9p/v9fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ static inline struct v9fs_session_info *v9fs_inode2v9ses(struct inode *inode)
return inode->i_sb->s_fs_info;
}

static inline struct v9fs_session_info *v9fs_dentry2v9ses(struct dentry *dentry)
static inline struct v9fs_session_info *v9fs_dentry2v9ses(const struct dentry *dentry)
{
return dentry->d_sb->s_fs_info;
}
Expand Down
26 changes: 24 additions & 2 deletions fs/9p/vfs_dentry.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ static void v9fs_dentry_release(struct dentry *dentry)
p9_fid_put(hlist_entry(p, struct p9_fid, dlist));
}

static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
{
struct p9_fid *fid;
struct inode *inode;
Expand Down Expand Up @@ -99,14 +99,36 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
return 1;
}

static int v9fs_lookup_revalidate(struct inode *dir, const struct qstr *name,
struct dentry *dentry, unsigned int flags)
{
return __v9fs_lookup_revalidate(dentry, flags);
}

static bool v9fs_dentry_unalias_trylock(const struct dentry *dentry)
{
struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
return down_write_trylock(&v9ses->rename_sem);
}

static void v9fs_dentry_unalias_unlock(const struct dentry *dentry)
{
struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
up_write(&v9ses->rename_sem);
}

const struct dentry_operations v9fs_cached_dentry_operations = {
.d_revalidate = v9fs_lookup_revalidate,
.d_weak_revalidate = v9fs_lookup_revalidate,
.d_weak_revalidate = __v9fs_lookup_revalidate,
.d_delete = v9fs_cached_dentry_delete,
.d_release = v9fs_dentry_release,
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
};

const struct dentry_operations v9fs_dentry_operations = {
.d_delete = always_delete_dentry,
.d_release = v9fs_dentry_release,
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
};
40 changes: 12 additions & 28 deletions fs/afs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
unsigned int flags);
static int afs_dir_open(struct inode *inode, struct file *file);
static int afs_readdir(struct file *file, struct dir_context *ctx);
static int afs_d_revalidate(struct dentry *dentry, unsigned int flags);
static int afs_d_revalidate(struct inode *dir, const struct qstr *name,
struct dentry *dentry, unsigned int flags);
static int afs_d_delete(const struct dentry *dentry);
static void afs_d_iput(struct dentry *dentry, struct inode *inode);
static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name, int nlen,
Expand Down Expand Up @@ -597,19 +598,19 @@ static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
* Do a lookup of a single name in a directory
* - just returns the FID the dentry name maps to if found
*/
static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
static int afs_do_lookup_one(struct inode *dir, const struct qstr *name,
struct afs_fid *fid,
afs_dataversion_t *_dir_version)
{
struct afs_super_info *as = dir->i_sb->s_fs_info;
struct afs_lookup_one_cookie cookie = {
.ctx.actor = afs_lookup_one_filldir,
.name = dentry->d_name,
.name = *name,
.fid.vid = as->volume->vid
};
int ret;

_enter("{%lu},%p{%pd},", dir->i_ino, dentry, dentry);
_enter("{%lu},{%.*s},", dir->i_ino, name->len, name->name);

/* search the directory */
ret = afs_dir_iterate(dir, &cookie.ctx, NULL, _dir_version);
Expand Down Expand Up @@ -1023,21 +1024,12 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
/*
* Check the validity of a dentry under RCU conditions.
*/
static int afs_d_revalidate_rcu(struct dentry *dentry)
static int afs_d_revalidate_rcu(struct afs_vnode *dvnode, struct dentry *dentry)
{
struct afs_vnode *dvnode;
struct dentry *parent;
struct inode *dir;
long dir_version, de_version;

_enter("%p", dentry);

/* Check the parent directory is still valid first. */
parent = READ_ONCE(dentry->d_parent);
dir = d_inode_rcu(parent);
if (!dir)
return -ECHILD;
dvnode = AFS_FS_I(dir);
if (test_bit(AFS_VNODE_DELETED, &dvnode->flags))
return -ECHILD;

Expand Down Expand Up @@ -1065,19 +1057,19 @@ static int afs_d_revalidate_rcu(struct dentry *dentry)
* - NOTE! the hit can be a negative hit too, so we can't assume we have an
* inode
*/
static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,
struct dentry *dentry, unsigned int flags)
{
struct afs_vnode *vnode, *dir;
struct afs_vnode *vnode, *dir = AFS_FS_I(parent_dir);
struct afs_fid fid;
struct dentry *parent;
struct inode *inode;
struct key *key;
afs_dataversion_t dir_version, invalid_before;
long de_version;
int ret;

if (flags & LOOKUP_RCU)
return afs_d_revalidate_rcu(dentry);
return afs_d_revalidate_rcu(dir, dentry);

if (d_really_is_positive(dentry)) {
vnode = AFS_FS_I(d_inode(dentry));
Expand All @@ -1092,14 +1084,9 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
if (IS_ERR(key))
key = NULL;

/* Hold the parent dentry so we can peer at it */
parent = dget_parent(dentry);
dir = AFS_FS_I(d_inode(parent));

/* validate the parent directory */
ret = afs_validate(dir, key);
if (ret == -ERESTARTSYS) {
dput(parent);
key_put(key);
return ret;
}
Expand Down Expand Up @@ -1127,7 +1114,7 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
afs_stat_v(dir, n_reval);

/* search the directory for this vnode */
ret = afs_do_lookup_one(&dir->netfs.inode, dentry, &fid, &dir_version);
ret = afs_do_lookup_one(&dir->netfs.inode, name, &fid, &dir_version);
switch (ret) {
case 0:
/* the filename maps to something */
Expand Down Expand Up @@ -1171,22 +1158,19 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
goto out_valid;

default:
_debug("failed to iterate dir %pd: %d",
parent, ret);
_debug("failed to iterate parent %pd2: %d", dentry, ret);
goto not_found;
}

out_valid:
dentry->d_fsdata = (void *)(unsigned long)dir_version;
out_valid_noupdate:
dput(parent);
key_put(key);
_leave(" = 1 [valid]");
return 1;

not_found:
_debug("dropping dentry %pd2", dentry);
dput(parent);
key_put(key);

_leave(" = 0 [bad]");
Expand Down
25 changes: 7 additions & 18 deletions fs/ceph/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1940,29 +1940,19 @@ static int dir_lease_is_valid(struct inode *dir, struct dentry *dentry,
/*
* Check if cached dentry can be trusted.
*/
static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
static int ceph_d_revalidate(struct inode *dir, const struct qstr *name,
struct dentry *dentry, unsigned int flags)
{
struct ceph_mds_client *mdsc = ceph_sb_to_fs_client(dentry->d_sb)->mdsc;
struct ceph_client *cl = mdsc->fsc->client;
int valid = 0;
struct dentry *parent;
struct inode *dir, *inode;
struct inode *inode;

valid = fscrypt_d_revalidate(dentry, flags);
valid = fscrypt_d_revalidate(dir, name, dentry, flags);
if (valid <= 0)
return valid;

if (flags & LOOKUP_RCU) {
parent = READ_ONCE(dentry->d_parent);
dir = d_inode_rcu(parent);
if (!dir)
return -ECHILD;
inode = d_inode_rcu(dentry);
} else {
parent = dget_parent(dentry);
dir = d_inode(parent);
inode = d_inode(dentry);
}
inode = d_inode_rcu(dentry);

doutc(cl, "%p '%pd' inode %p offset 0x%llx nokey %d\n",
dentry, dentry, inode, ceph_dentry(dentry)->offset,
Expand Down Expand Up @@ -2008,6 +1998,8 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
req->r_parent = dir;
ihold(dir);

req->r_dname = name;

mask = CEPH_STAT_CAP_INODE | CEPH_CAP_AUTH_SHARED;
if (ceph_security_xattr_wanted(dir))
mask |= CEPH_CAP_XATTR_SHARED;
Expand Down Expand Up @@ -2038,9 +2030,6 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
doutc(cl, "%p '%pd' %s\n", dentry, dentry, valid ? "valid" : "invalid");
if (!valid)
ceph_dir_clear_complete(dir);

if (!(flags & LOOKUP_RCU))
dput(parent);
return valid;
}

Expand Down
9 changes: 6 additions & 3 deletions fs/ceph/mds_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -2621,6 +2621,7 @@ static u8 *get_fscrypt_altname(const struct ceph_mds_request *req, u32 *plen)
{
struct inode *dir = req->r_parent;
struct dentry *dentry = req->r_dentry;
const struct qstr *name = req->r_dname;
u8 *cryptbuf = NULL;
u32 len = 0;
int ret = 0;
Expand All @@ -2641,8 +2642,10 @@ static u8 *get_fscrypt_altname(const struct ceph_mds_request *req, u32 *plen)
if (!fscrypt_has_encryption_key(dir))
goto success;

if (!fscrypt_fname_encrypted_size(dir, dentry->d_name.len, NAME_MAX,
&len)) {
if (!name)
name = &dentry->d_name;

if (!fscrypt_fname_encrypted_size(dir, name->len, NAME_MAX, &len)) {
WARN_ON_ONCE(1);
return ERR_PTR(-ENAMETOOLONG);
}
Expand All @@ -2657,7 +2660,7 @@ static u8 *get_fscrypt_altname(const struct ceph_mds_request *req, u32 *plen)
if (!cryptbuf)
return ERR_PTR(-ENOMEM);

ret = fscrypt_fname_encrypt(dir, &dentry->d_name, cryptbuf, len);
ret = fscrypt_fname_encrypt(dir, name, cryptbuf, len);
if (ret) {
kfree(cryptbuf);
return ERR_PTR(ret);
Expand Down
2 changes: 2 additions & 0 deletions fs/ceph/mds_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ struct ceph_mds_request {
struct inode *r_target_inode; /* resulting inode */
struct inode *r_new_inode; /* new inode (for creates) */

const struct qstr *r_dname; /* stable name (for ->d_revalidate) */

#define CEPH_MDS_R_DIRECT_IS_HASH (1) /* r_direct_hash is valid */
#define CEPH_MDS_R_ABORTED (2) /* call was aborted */
#define CEPH_MDS_R_GOT_UNSAFE (3) /* got an unsafe reply */
Expand Down
3 changes: 2 additions & 1 deletion fs/coda/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ static int coda_readdir(struct file *coda_file, struct dir_context *ctx)
}

/* called when a cache lookup succeeds */
static int coda_dentry_revalidate(struct dentry *de, unsigned int flags)
static int coda_dentry_revalidate(struct inode *dir, const struct qstr *name,
struct dentry *de, unsigned int flags)
{
struct inode *inode;
struct coda_inode_info *cii;
Expand Down
Loading

0 comments on commit d3d90cc

Please sign in to comment.