On Fri, Nov 17, 2023 at 11:19:51AM +0800, xiubli@redhat.com wrote:
From: Xiubo Li xiubli@redhat.com
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@vger.kernel.org Reported-by: Al Viro viro@zeniv.linux.org.uk 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@kernel.org Signed-off-by: Xiubo Li xiubli@redhat.com
- 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);