On 11/17/23 12:45, Al Viro wrote:
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);
Yeah, you are right.
Will update it.
Thanks
- Xiubo
