]> git.apps.os.sepia.ceph.com Git - ceph-client.git/commitdiff
ceph: fix client race condition where r_parent becomes stale before sending message wip-tls-tracing-parent-fix
authorAlex Markuze <amarkuze@redhat.com>
Wed, 30 Jul 2025 15:03:26 +0000 (15:03 +0000)
committerAlex Markuze <amarkuze@redhat.com>
Thu, 31 Jul 2025 11:08:07 +0000 (11:08 +0000)
When the parent directory's i_rwsem is not locked, req->r_parent may become
stale due to concurrent operations (e.g. rename) between dentry lookup and
message creation. Validate that r_parent matches the encoded parent inode
and update to the correct inode if a mismatch is detected.

fs/ceph/inode.c

index 814f9e9656a0d71972e6cb6b94be2acd73fc89fd..49fb1e3a02e8ae0416bcbcae3673cdf6ee92b985 100644 (file)
@@ -56,6 +56,46 @@ static int ceph_set_ino_cb(struct inode *inode, void *data)
        return 0;
 }
 
+/*
+ * Validate that the directory inode referenced by @req->r_parent matches the
+ * inode number and snapshot id contained in the reply's directory record.  If
+ * they do not match – which can theoretically happen if the parent dentry was
+ * moved between the time the request was issued and the reply arrived – fall
+ * back to looking up the correct inode in the inode cache.
+ *
+ * A reference is *always* returned.  Callers that receive a different inode
+ * than the original @parent are responsible for dropping the extra reference
+ * once the reply has been processed.
+ */
+static struct inode *ceph_get_reply_dir(struct super_block *sb,
+                                       struct inode *parent,
+                                       struct ceph_mds_reply_info_parsed *rinfo)
+{
+    struct ceph_vino vino;
+
+    if (unlikely(!rinfo->diri.in))
+        return parent; /* nothing to compare against */
+
+    /* If we didn't have a cached parent inode to begin with, just bail out. */
+    if (!parent)
+        return NULL;
+
+    vino.ino  = le64_to_cpu(rinfo->diri.in->ino);
+    vino.snap = le64_to_cpu(rinfo->diri.in->snapid);
+
+    if (likely(parent && ceph_ino(parent) == vino.ino && ceph_snap(parent) == vino.snap))
+        return parent; /* matches – use the original reference */
+
+    /* Mismatch – this should be rare.  Emit a WARN and obtain the correct inode. */
+    WARN(1, "ceph: reply dir mismatch (parent %s %llx.%llx reply %llx.%llx)\n",
+         parent ? "valid" : "NULL",
+         parent ? ceph_ino(parent) : 0ULL,
+         parent ? ceph_snap(parent) : 0ULL,
+         vino.ino, vino.snap);
+
+    return ceph_get_inode(sb, vino, NULL);
+}
+
 /**
  * ceph_new_inode - allocate a new inode in advance of an expected create
  * @dir: parent directory for new inode
@@ -1548,8 +1588,8 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
        }
 
        if (rinfo->head->is_dentry) {
-               struct inode *dir = req->r_parent;
-
+               /* r_parent may be stale, in cases when R_PARENT_LOCKED is not set, so we need to get the correct inode */
+               struct inode *dir = ceph_get_reply_dir(sb, req->r_parent, rinfo);
                if (dir) {
                        err = ceph_fill_inode(dir, NULL, &rinfo->diri,
                                              rinfo->dirfrag, session, -1,