os/,osd/: restructure the rados name length check

Enforce locator length vs the max name length and also
introduce a namespace length limit.

In addition to these checks, also pass the head object to the
ObjectStore implementation to validate.  This allows LFNIndex to account
for the idiosyncracies of its filename escaping and for different xattr
value max sizes.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index 5d1a6e8c8b08cce6ced22ee572039a8475c3461e..ef56656575f96649dca478047dd313372fbca62f 100644 (file)
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -869,6 +869,7 @@ OPTION(osd_mon_shutdown_timeout, OPT_DOUBLE, 5)
 
 OPTION(osd_max_object_size, OPT_U64, 100*1024L*1024L*1024L) // OSD's maximum object size
 OPTION(osd_max_object_name_len, OPT_U32, 2048) // max rados object name len
+OPTION(osd_max_object_namespace_len, OPT_U32, 256) // max rados object namespace len
 OPTION(osd_max_attr_name_len, OPT_U32, 100)    // max rados attr name len; cannot go higher than 100 chars for file system backends
 OPTION(osd_max_attr_size, OPT_U64, 0)
 
@@ -1026,6 +1027,18 @@ OPTION(filestore_max_inline_xattrs_xfs, OPT_U32, 10)
 OPTION(filestore_max_inline_xattrs_btrfs, OPT_U32, 10)
 OPTION(filestore_max_inline_xattrs_other, OPT_U32, 2)
 
+// max xattr value size
+OPTION(filestore_max_xattr_value_size, OPT_U32, 0)     //Override
+OPTION(filestore_max_xattr_value_size_xfs, OPT_U32, 64<<10)
+OPTION(filestore_max_xattr_value_size_btrfs, OPT_U32, 64<<10)
+// ext4 allows 4k xattrs total including some smallish extra fields and the
+// keys.  We're allowing 2 512 inline attrs in addition some some filestore
+// replay attrs.  After accounting for those, we still need to fit up to
+// two attrs of this value.  That means we need this value to be around 1k
+// to be safe.  This is hacky, but it's not worth complicating the code
+// to work around ext4's total xattr limit.
+OPTION(filestore_max_xattr_value_size_other, OPT_U32, 1<<10)
+
 OPTION(filestore_sloppy_crc, OPT_BOOL, false)         // track sloppy crcs
 OPTION(filestore_sloppy_crc_block_size, OPT_INT, 65536)
 

diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
index c561d31071f0ac84d3e9481a45256f92b4390c90..71dcd8077824cbca8791a5741440554ae45aa074 100644 (file)
--- a/src/os/ObjectStore.h
+++ b/src/os/ObjectStore.h
@@ -1926,7 +1926,15 @@ public:
   virtual int fsck() {
     return -EOPNOTSUPP;
   }
-  virtual unsigned get_max_object_name_length() = 0;
+
+  /**
+   * Returns 0 if the hobject is valid, -error otherwise
+   *
+   * Errors:
+   * -ENAMETOOLONG: locator/namespace/name too large
+   */
+  virtual int validate_hobject_key(const hobject_t &obj) const = 0;
+
   virtual unsigned get_max_attr_name_length() = 0;
   virtual int mkfs() = 0;  // wipe
   virtual int mkjournal() = 0; // journal only

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 828dd914ff165a6f4310e3bd31996be0705502bf..91aec621603bc71d6e2e71dd8a008e5d07aecd23 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -667,8 +667,8 @@ public:
 
   int fsck() override;
 
-  unsigned get_max_object_name_length() override {
-    return 4096;
+  int validate_hobject_key(const hobject_t &obj) const override {
+    return 0;
   }
   unsigned get_max_attr_name_length() override {
     return 256;  // arbitrary; there is no real limit internally

diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index 97b952ef67dad8817d50b72d78f29c1761af3d55..27465248ba97bd5f4bd125233aae3e4530dcb196 100644 (file)
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -123,6 +123,12 @@ static CompatSet get_fs_supported_compat_set() {
   return compat;
 }
 
+int FileStore::validate_hobject_key(const hobject_t &obj) const
+{
+  unsigned len = LFNIndex::get_max_escaped_name_len(obj);
+  return len > m_filestore_max_xattr_value_size ? -ENAMETOOLONG : 0;
+}
+
 int FileStore::get_block_device_fsid(const string& path, uuid_d *fsid)
 {
   // make sure we don't try to use aio or direct_io (and get annoying
@@ -559,7 +565,8 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, osflagbit
   m_filestore_max_alloc_hint_size(g_conf->filestore_max_alloc_hint_size),
   m_fs_type(0),
   m_filestore_max_inline_xattr_size(0),
-  m_filestore_max_inline_xattrs(0)
+  m_filestore_max_inline_xattrs(0),
+  m_filestore_max_xattr_value_size(0)
 {
   m_filestore_kill_at.set(g_conf->filestore_kill_at);
   for (int i = 0; i < m_ondisk_finisher_num; ++i) {
@@ -5663,21 +5670,25 @@ void FileStore::set_xattr_limits_via_conf()
 {
   uint32_t fs_xattr_size;
   uint32_t fs_xattrs;
+  uint32_t fs_xattr_max_value_size;
 
   switch (m_fs_type) {
 #if defined(__linux__)
   case XFS_SUPER_MAGIC:
     fs_xattr_size = g_conf->filestore_max_inline_xattr_size_xfs;
     fs_xattrs = g_conf->filestore_max_inline_xattrs_xfs;
+    fs_xattr_max_value_size = g_conf->filestore_max_xattr_value_size_xfs;
     break;
   case BTRFS_SUPER_MAGIC:
     fs_xattr_size = g_conf->filestore_max_inline_xattr_size_btrfs;
     fs_xattrs = g_conf->filestore_max_inline_xattrs_btrfs;
+    fs_xattr_max_value_size = g_conf->filestore_max_xattr_value_size_btrfs;
     break;
 #endif
   default:
     fs_xattr_size = g_conf->filestore_max_inline_xattr_size_other;
     fs_xattrs = g_conf->filestore_max_inline_xattrs_other;
+    fs_xattr_max_value_size = g_conf->filestore_max_xattr_value_size_other;
     break;
   }
 
@@ -5692,6 +5703,12 @@ void FileStore::set_xattr_limits_via_conf()
     m_filestore_max_inline_xattrs = g_conf->filestore_max_inline_xattrs;
   else
     m_filestore_max_inline_xattrs = fs_xattrs;
+
+  // Use override value if set
+  if (g_conf->filestore_max_xattr_value_size)
+    m_filestore_max_xattr_value_size = g_conf->filestore_max_xattr_value_size;
+  else
+    m_filestore_max_xattr_value_size = fs_xattr_max_value_size;
 }
 
 // -- FSSuperblock --

diff --git a/src/os/filestore/FileStore.h b/src/os/filestore/FileStore.h
index d81f8b0691b02917216157902eb63f04eb7baab8..a5cd75d5e5304145701cd3030cd14eba24b19e1e 100644 (file)
--- a/src/os/filestore/FileStore.h
+++ b/src/os/filestore/FileStore.h
@@ -432,10 +432,9 @@ public:
   int write_op_seq(int, uint64_t seq);
   int mount();
   int umount();
-  unsigned get_max_object_name_length() {
-    // not safe for all file systems, btw!  use the tunable to limit this.
-    return 4096;
-  }
+
+  int validate_hobject_key(const hobject_t &obj) const override;
+
   unsigned get_max_attr_name_length() {
     // xattr limit is 128; leave room for our prefixes (user.ceph._),
     // some margin, and cap at 100
@@ -739,6 +738,7 @@ private:
   void set_xattr_limits_via_conf();
   uint32_t m_filestore_max_inline_xattr_size;
   uint32_t m_filestore_max_inline_xattrs;
+  uint32_t m_filestore_max_xattr_value_size;
 
   FSSuperblock superblock;
 

diff --git a/src/os/filestore/LFNIndex.cc b/src/os/filestore/LFNIndex.cc
index 47436ea444d02d4e8ad301ca49a1e8b34e255829..4842837c305394867bd6bad3f07b10a0e6903fa0 100644 (file)
--- a/src/os/filestore/LFNIndex.cc
+++ b/src/os/filestore/LFNIndex.cc
@@ -74,6 +74,14 @@ struct FDCloser {
 
 /* Public methods */
 
+uint64_t LFNIndex::get_max_escaped_name_len(const hobject_t &obj)
+{
+  ghobject_t ghobj(obj);
+  ghobj.shard_id = shard_id_t(0);
+  ghobj.generation = 0;
+  ghobj.hobj.snap = 0;
+  return lfn_generate_object_name_current(ghobj).size();
+}
 
 int LFNIndex::init()
 {
@@ -621,13 +629,8 @@ static void append_escaped(string::const_iterator begin,
   }
 }
 
-string LFNIndex::lfn_generate_object_name(const ghobject_t &oid)
+string LFNIndex::lfn_generate_object_name_current(const ghobject_t &oid)
 {
-  if (index_version == HASH_INDEX_TAG)
-    return lfn_generate_object_name_keyless(oid);
-  if (index_version == HASH_INDEX_TAG_2)
-    return lfn_generate_object_name_poolless(oid);
-
   string full_name;
   string::const_iterator i = oid.hobj.oid.name.begin();
   if (oid.hobj.oid.name.substr(0, 4) == "DIR_") {

diff --git a/src/os/filestore/LFNIndex.h b/src/os/filestore/LFNIndex.h
index 1cf4f0b8e7f0dfc60df869691644d47583134275..e84fc269fd3e28c06a64fc5369a8f68a61e496f8 100644 (file)
--- a/src/os/filestore/LFNIndex.h
+++ b/src/os/filestore/LFNIndex.h
@@ -212,6 +212,11 @@ public:
       );
   }
 
+  /**
+   * Returns the length of the longest escaped name which could result
+   * from any clone, shard, or rollback object of this object
+   */
+  static uint64_t get_max_escaped_name_len(const hobject_t &obj);
 
 protected:
   virtual int _init() = 0;
@@ -480,10 +485,22 @@ private:
     ); ///< @return Generated object name.
 
   /// Generate object name
-  string lfn_generate_object_name(
+  static string lfn_generate_object_name_current(
     const ghobject_t &oid ///< [in] Object for which to generate.
     ); ///< @return Generated object name.
 
+  /// Generate object name
+  string lfn_generate_object_name(
+    const ghobject_t &oid ///< [in] Object for which to generate.
+    ) {
+    if (index_version == HASH_INDEX_TAG)
+      return lfn_generate_object_name_keyless(oid);
+    if (index_version == HASH_INDEX_TAG_2)
+      return lfn_generate_object_name_poolless(oid);
+    else
+      return lfn_generate_object_name_current(oid);
+  } ///< @return Generated object name.
+
   /// Parse object name
   bool lfn_parse_object_name_keyless(
     const string &long_name, ///< [in] Name to parse

diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h
index 4b0066331deeac3ee3dc17dce121fc2acadea7fd..09483de46be6d1e75a85644047e065dd467f03ca 100644 (file)
--- a/src/os/kstore/KStore.h
+++ b/src/os/kstore/KStore.h
@@ -411,8 +411,9 @@ public:
 
   int fsck();
 
-  unsigned get_max_object_name_length() {
-    return 4096;
+
+  int validate_hobject_key(const hobject_t &obj) const override {
+    return 0;
   }
   unsigned get_max_attr_name_length() {
     return 256;  // arbitrary; there is no real limit internally

diff --git a/src/os/memstore/MemStore.h b/src/os/memstore/MemStore.h
index 2d809f3384f954ce926f6df97ee31663672f59de..64f9afc16fae5a3c5d928b11f8ea34d9398bf955 100644 (file)
--- a/src/os/memstore/MemStore.h
+++ b/src/os/memstore/MemStore.h
@@ -365,8 +365,8 @@ public:
   int mount();
   int umount();
 
-  unsigned get_max_object_name_length() {
-    return 4096;
+  int validate_hobject_key(const hobject_t &obj) const override {
+    return 0;
   }
   unsigned get_max_attr_name_length() {
     return 256;  // arbitrary; there is no real limit internally

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 3958f8942ea579e02a9fd291e387325e16cf6bb6..4f6cf445bc1f3abcd819c3e5733ec50a431e157c 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -1627,16 +1627,41 @@ void ReplicatedPG::do_op(OpRequestRef& op)
     return;
   }
 
+  hobject_t head(m->get_oid(), m->get_object_locator().key,
+                CEPH_NOSNAP, m->get_pg().ps(),
+                info.pgid.pool(), m->get_object_locator().nspace);
+
   // object name too long?
-  unsigned max_name_len = MIN(g_conf->osd_max_object_name_len,
-                              osd->osd->store->get_max_object_name_length());
-  if (m->get_oid().name.size() > max_name_len) {
-    dout(4) << "do_op '" << m->get_oid().name << "' is longer than "
-            << max_name_len << " bytes" << dendl;
+  if (m->get_oid().name.size() > g_conf->osd_max_object_name_len) {
+    dout(4) << "do_op name is longer than "
+            << g_conf->osd_max_object_name_len
+           << " bytes" << dendl;
+    osd->reply_op_error(op, -ENAMETOOLONG);
+    return;
+  }
+  if (m->get_object_locator().key.size() > g_conf->osd_max_object_name_len) {
+    dout(4) << "do_op locator is longer than "
+            << g_conf->osd_max_object_name_len
+           << " bytes" << dendl;
+    osd->reply_op_error(op, -ENAMETOOLONG);
+    return;
+  }
+  if (m->get_object_locator().nspace.size() >
+      g_conf->osd_max_object_namespace_len) {
+    dout(4) << "do_op namespace is longer than "
+            << g_conf->osd_max_object_namespace_len
+           << " bytes" << dendl;
     osd->reply_op_error(op, -ENAMETOOLONG);
     return;
   }
 
+  if (int r = osd->store->validate_hobject_key(head)) {
+    dout(4) << "do_op object " << head << " invalid for backing store: "
+           << r << dendl;
+    osd->reply_op_error(op, r);
+    return;
+  }
+
   // blacklisted?
   if (get_osdmap()->is_blacklisted(m->get_source_addr())) {
     dout(10) << "do_op " << m->get_source_addr() << " is blacklisted" << dendl;
@@ -1702,11 +1727,6 @@ void ReplicatedPG::do_op(OpRequestRef& op)
           << " flags " << ceph_osd_flag_string(m->get_flags())
           << dendl;
 
-  hobject_t head(m->get_oid(), m->get_object_locator().key,
-                CEPH_NOSNAP, m->get_pg().ps(),
-                info.pgid.pool(), m->get_object_locator().nspace);
-
-
   if (write_ordered &&
       scrubber.write_blocked_by_scrub(head, get_sort_bitwise())) {
     dout(20) << __func__ << ": waiting for scrub" << dendl;