mds: don't shallow copy when decoding xattr map

Otherwise inodes' xattr maps may reference a large shared buffer
(from omap fetch or journal read). If mds trims and loads inode
repeatly, each inode can reference different large buffer in the
worst case.

Fixes: https://tracker.ceph.com/issues/45090
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index 0de21439be1886599007bcc9974459a768e71f98..84e7a83ea8aed40ab2759fa13bd7ca35a3db661f 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -1541,7 +1541,7 @@ void InodeStoreBase::decode_bare(bufferlist::const_iterator &bl,
     symlink = std::string_view(tmp);
   }
   decode(dirfragtree, bl);
-  decode(xattrs, bl);
+  decode_noshare(xattrs, bl);
   decode(snap_blob, bl);
 
   decode(old_inodes, bl);
@@ -1965,7 +1965,7 @@ void CInode::decode_lock_ixattr(bufferlist::const_iterator& p)
   utime_t tm;
   decode(tm, p);
   if (inode.ctime < tm) inode.ctime = tm;
-  decode(xattrs, p);
+  decode_noshare(xattrs, p);
   DECODE_FINISH(p);
 }
 
@@ -4066,7 +4066,7 @@ void CInode::_decode_base(bufferlist::const_iterator& p)
     symlink = std::string_view(tmp);
   }
   decode(dirfragtree, p);
-  decode(xattrs, p);
+  decode_noshare(xattrs, p);
   decode(old_inodes, p);
   decode(damage_flags, p);
   decode_snap(p);

diff --git a/src/mds/CInode.h b/src/mds/CInode.h
index e76fd2b3f30720cca7003401225aa9fe6ff19c8b..5c84c533f6bd72d0e89ea73a7519cfd53d0a48c0 100644 (file)
--- a/src/mds/CInode.h
+++ b/src/mds/CInode.h
@@ -108,6 +108,12 @@ public:
   damage_flags_t damage_flags = 0;
 };
 
+inline void decode_noshare(InodeStoreBase::mempool_xattr_map& xattrs,
+                          ceph::buffer::list::const_iterator &p)
+{
+  decode_noshare<mempool::mds_co::pool_allocator>(xattrs, p);
+}
+
 class InodeStore : public InodeStoreBase {
 public:
   void encode(ceph::buffer::list &bl, uint64_t features) const {

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index b811db4870cf315f59bcc160d4bc5ede125453fd..4594d97dd0d6aad1be182444b9e714600f84b891 100644 (file)
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -3833,7 +3833,7 @@ bool Locker::_do_cap_update(CInode *in, Capability *cap,
     dout(7) << " xattrs v" << pi.inode.xattr_version << " -> " << m->head.xattr_version << dendl;
     pi.inode.xattr_version = m->head.xattr_version;
     auto p = m->xattrbl.cbegin();
-    decode(*pi.xattrs, p);
+    decode_noshare(*pi.xattrs, p);
     wrlock_force(&in->xattrlock, mut);
   }
   

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index ae5996e086f35a20c96095e6d322684fcbcd9252..1c487e714c431e0d59181931c9181b32ad971e0a 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -3302,7 +3302,7 @@ CInode* Server::prepare_new_inode(MDRequestRef& mdr, CDir *dir, inodeno_t useino
 
     // xattrs on new inode?
     CInode::mempool_xattr_map xattrs;
-    decode(xattrs, p);
+    decode_noshare(xattrs, p);
     for (const auto &p : xattrs) {
       dout(10) << "prepare_new_inode setting xattr " << p.first << dendl;
       auto em = in->xattrs.emplace(std::piecewise_construct, std::forward_as_tuple(p.first), std::forward_as_tuple(p.second));

diff --git a/src/mds/journal.cc b/src/mds/journal.cc
index 14966f14ce153847b78a94da0e2216ea2b18e37b..aa783f5f0975b81c8ce0c6a41d73269798838ab9 100644 (file)
--- a/src/mds/journal.cc
+++ b/src/mds/journal.cc
@@ -428,7 +428,7 @@ void EMetaBlob::fullbit::decode(bufferlist::const_iterator &bl) {
   decode(dnlast, bl);
   decode(dnv, bl);
   decode(inode, bl);
-  decode(xattrs, bl);
+  decode_noshare(xattrs, bl);
   if (inode.is_symlink())
     decode(symlink, bl);
   if (inode.is_dir()) {

diff --git a/src/mds/mdstypes.h b/src/mds/mdstypes.h
index 3f46860e78b562ad00dc19f8d957d51c5ef5a782..1d138c7c55a6c431f1ecfc2493440c15650a2d65 100644 (file)
--- a/src/mds/mdstypes.h
+++ b/src/mds/mdstypes.h
@@ -997,6 +997,20 @@ using xattr_map = compact_map<alloc_string<Allocator>,
                              Allocator<std::pair<const alloc_string<Allocator>,
                                                  ceph::bufferptr>>>; // FIXME bufferptr not in mempool
 
+template<template<typename> class Allocator>
+inline void decode_noshare(xattr_map<Allocator>& xattrs, ceph::buffer::list::const_iterator &p)
+{
+  __u32 n;
+  decode(n, p);
+  while (n-- > 0) {
+    alloc_string<Allocator> key;
+    decode(key, p);
+    __u32 len;
+    decode(len, p);
+    p.copy_deep(len, xattrs[key]);
+  }
+}
+
 template<template<typename> class Allocator = std::allocator>
 struct old_inode_t {
   snapid_t first;
@@ -1026,7 +1040,7 @@ void old_inode_t<Allocator>::decode(ceph::buffer::list::const_iterator& bl)
   DECODE_START_LEGACY_COMPAT_LEN(2, 2, 2, bl);
   decode(first, bl);
   decode(inode, bl);
-  decode(xattrs, bl);
+  decode_noshare<Allocator>(xattrs, bl);
   DECODE_FINISH(bl);
 }