From: Samuel Just <sjust@redhat.com>
Date: Thu, 18 Apr 2024 22:19:31 +0000 (-0700)
Subject: os/: modify getattrs to clear attrs out param before populating
X-Git-Tag: v19.1.0~19^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F57006%2Fhead;p=ceph.git

os/: modify getattrs to clear attrs out param before populating

Passing in a non-empty map would otherwise exhibit quite unexpected
behavior.  For the bufferptr overload, any preexisting entries would
not be overwritten due to how std::map::emplace behaves.  For the
bufferlist overload, it would result in appending to any pre-existing
entries.

The prior commit cleans up one such inadvertent caller which resulted
in the below bug.

Fixes: https://tracker.ceph.com/issues/65185
Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit 915f92ba9dc8e192bdcebcb92e7d06a93a401f5d)
---

diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
index 302be387fae6..4889b7152679 100644
--- a/src/os/ObjectStore.h
+++ b/src/os/ObjectStore.h
@@ -611,7 +611,7 @@ public:
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   virtual int getattrs(CollectionHandle &c, const ghobject_t& oid,
@@ -622,13 +622,14 @@ public:
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   int getattrs(CollectionHandle &c, const ghobject_t& oid,
 	       std::map<std::string,ceph::buffer::list,std::less<>>& aset) {
     std::map<std::string,ceph::buffer::ptr,std::less<>> bmap;
     int r = getattrs(c, oid, bmap);
+    aset.clear();
     for (auto i = bmap.begin(); i != bmap.end(); ++i) {
       aset[i->first].append(i->second);
     }
diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 7d02b1551e0f..dda2b0b1e63a 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -12426,6 +12426,7 @@ int BlueStore::getattrs(
       r = -ENOENT;
       goto out;
     }
+    aset.clear();
     for (auto& i : o->onode.attrs) {
       aset.emplace(i.first.c_str(), i.second);
     }