mds: include encoded stray inode when sending dentry unlink message to replicas

The series of events that lead to unaccessible dentries is as follows (requires some
hardlinked files, directory pinning and path restricted caps).

Assume 3 hardlinked files:

       d0/f0 <-- primary link
       d1/h1
       d1/h2

with multiple active MDSs -- d0 pinned to rank-0 and d1 to rank-1. Reproducing
this requires deleting a non-primary link first followed by deleting the primary
link and lastly the other non-primary link. If one if unlucky, the last delete
fails with "Permission denied" error. On the MDS side, this is what happens:

Unlinking the first non-primary link would discover the remote inode and link
it in dentry as part of lookup. Unlink would send a remote unlink operation
(_link_remote(op-unlink)) to the auth mds of the remote inode. Now the other
unlinks:

               rank0               |                 rank1
                                   |
                x-----<-----unlink: #unlink(d0/f0)
                |                  |
                v                  |
         _unlink_local()           |
                |                  |
                v                  |
                |                  |
   x ---send_dentry_unlink()       |
   |   reply_client_request()      |
   |                               |
   |                       unlink: #lookup(d1/h2)----x
   |                               |                 |
   |                               |                 v
   |                               |       MDCache::path_traverse()
   v                               |       (uses linked remote inode,
   |                               |       which is not a stray inode)-----x
   |                               |                                       |
   x-------------->----------------|---------->------x                     v
         (dentry_unlink msg)       |                 |                     |
                                   |                 v                     |
                                   |       handle_dentry_unlink()          |
                                   |    (relinks inode to stray dentry)    |
                                   |                                       |
                                   |                 x---------------------x
                                   |                 |
                                   |                 v
                                   |       dispatch_client_request()
                                   |        (uses linked inode under
                                   |  stray dentry, no reintegration check)
                                   |                 |
                                   |                 v
                                   |      SessionMap::check_access()
                                   |      (parent is a stray dir,
                                   |    but stray_prior_path is empty)

One possible fix could be to fix SessionMap::check_access() to build the
inode path for a inode under stray directory, however, the inode path
would be on the form "#<inode>" and it does not look like there is a
good way to figure out what it was (as seen in inode dump).

Sending the (encoded) inode as part of `dentry_unlink' message seems to
get handled well -- decode_replica_inode() handles the case where the
inode already existed (in MDCache::inode_map) and the auth mds sending
the stray inode to its replicas doesn't seem to cause trouble. (I think
it didn't do this since the replicas have the inode anyway, but in this
case this inode is a _bit_ out of sync with what exists in the auth).

Fixes: http://tracker.ceph.com/issues/54046
Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit 59e01da10695c2ccff2623c09e78a9d572e64235)

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index ef6d3c4c82ae3a93fe1f775cdc605db371fe36ac..e28744bfa35e3faa7258bd36a16e7d4d809caac6 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -10787,7 +10787,7 @@ void MDCache::decode_replica_inode(CInode *&in, bufferlist::const_iterator& p, C
 void MDCache::encode_replica_stray(CDentry *straydn, mds_rank_t who, bufferlist& bl)
 {
   ceph_assert(straydn->get_num_auth_pins());
-  ENCODE_START(1, 1, bl);
+  ENCODE_START(2, 1, bl);
   uint64_t features = mds->mdsmap->get_up_features();
   encode_replica_inode(get_myin(), who, bl, features);
   encode_replica_dir(straydn->get_dir()->inode->get_parent_dn()->get_dir(), who, bl);
@@ -10795,15 +10795,18 @@ void MDCache::encode_replica_stray(CDentry *straydn, mds_rank_t who, bufferlist&
   encode_replica_inode(straydn->get_dir()->inode, who, bl, features);
   encode_replica_dir(straydn->get_dir(), who, bl);
   encode_replica_dentry(straydn, who, bl);
+  if (!straydn->get_projected_linkage()->is_null()) {
+    encode_replica_inode(straydn->get_projected_linkage()->get_inode(), who, bl, features);
+  }
   ENCODE_FINISH(bl);
 }
    
-void MDCache::decode_replica_stray(CDentry *&straydn, const bufferlist &bl, mds_rank_t from)
+void MDCache::decode_replica_stray(CDentry *&straydn, CInode **in, const bufferlist &bl, mds_rank_t from)
 {
   MDSContext::vec finished;
   auto p = bl.cbegin();
 
-  DECODE_START(1, p);
+  DECODE_START(2, p);
   CInode *mdsin = nullptr;
   decode_replica_inode(mdsin, p, NULL, finished);
   CDir *mdsdir = nullptr;
@@ -10816,6 +10819,9 @@ void MDCache::decode_replica_stray(CDentry *&straydn, const bufferlist &bl, mds_
   decode_replica_dir(straydir, p, strayin, from, finished);
 
   decode_replica_dentry(straydn, p, straydir, finished);
+  if (struct_v >= 2 && in) {
+    decode_replica_inode(*in, p, straydn, finished);
+  }
   if (!finished.empty())
     mds->queue_waiters(finished);
   DECODE_FINISH(p);
@@ -11047,8 +11053,9 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
 {
   // straydn
   CDentry *straydn = nullptr;
+  CInode *strayin = nullptr;
   if (m->straybl.length())
-    decode_replica_stray(straydn, m->straybl, mds_rank_t(m->get_source().num()));
+    decode_replica_stray(straydn, &strayin, m->straybl, mds_rank_t(m->get_source().num()));
 
   CDir *dir = get_dirfrag(m->get_dirfrag());
   if (!dir) {

diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h
index e08168b714170b0b26829950046d7dc83e8ea549..f5b4e2a06124da80789da1a622c047067f1eb3d0 100644 (file)
--- a/src/mds/MDCache.h
+++ b/src/mds/MDCache.h
@@ -873,7 +873,7 @@ class MDCache {
   void decode_replica_inode(CInode *&in, bufferlist::const_iterator& p, CDentry *dn, MDSContext::vec& finished);
 
   void encode_replica_stray(CDentry *straydn, mds_rank_t who, bufferlist& bl);
-  void decode_replica_stray(CDentry *&straydn, const bufferlist &bl, mds_rank_t from);
+  void decode_replica_stray(CDentry *&straydn, CInode **in, const bufferlist &bl, mds_rank_t from);
 
   // -- namespace --
   void encode_remote_dentry_link(CDentry::linkage_t *dnl, bufferlist& bl);

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index e6f7e864fb41075870512325cda987ce7201e0c6..f7388a62a5ef016d1dd45cef2d41b20e6026b31d 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -2698,7 +2698,7 @@ void Server::handle_peer_request(const cref_t<MMDSPeerRequest> &m)
 
   CDentry *straydn = NULL;
   if (m->straybl.length() > 0) {
-    mdcache->decode_replica_stray(straydn, m->straybl, from);
+    mdcache->decode_replica_stray(straydn, nullptr, m->straybl, from);
     ceph_assert(straydn);
     m->straybl.clear();
   }