On Fri, Nov 17, 2023 at 11:19:51AM +0800, xiubli@xxxxxxxxxx wrote: > From: Xiubo Li <xiubli@xxxxxxxxxx> > > The lock order is incorrect between denty and its parent, we should > always make sure that the parent get the lock first. > > Switch to use the 'dget_parent()' to get the parent dentry and also > keep holding the parent until the corresponding inode is not being > refereenced. > > Cc: stable@xxxxxxxxxxxxxxx > Reported-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > URL: https://www.spinics.net/lists/ceph-devel/msg58622.html > Fixes: adf0d68701c7 ("ceph: fix unsafe dcache access in ceph_encode_dentry_release") > Cc: Jeff Layton <jlayton@xxxxxxxxxx> > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > + if (!dir) { > + parent = dget_parent(dentry); > + dir = d_inode(parent); > + } It's never actually called with dir == NULL. Not since 2016. All you need is this, _maybe_ with BUG_ON(!dir); added in the beginning of the function. diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 2c0b8dc3dd0d..22d6ea97938f 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -4887,7 +4887,6 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry, struct inode *dir, int mds, int drop, int unless) { - struct dentry *parent = NULL; struct ceph_mds_request_release *rel = *p; struct ceph_dentry_info *di = ceph_dentry(dentry); struct ceph_client *cl; @@ -4903,14 +4902,9 @@ int ceph_encode_dentry_release(void **p, struct dentry *dentry, spin_lock(&dentry->d_lock); if (di->lease_session && di->lease_session->s_mds == mds) force = 1; - if (!dir) { - parent = dget(dentry->d_parent); - dir = d_inode(parent); - } spin_unlock(&dentry->d_lock); ret = ceph_encode_inode_release(p, dir, mds, drop, unless, force); - dput(parent); cl = ceph_inode_to_client(dir); spin_lock(&dentry->d_lock);
