os/FileStore: fix fiemap double-free(s)

If GenericFileStoreBackend::do_fiemap returns anything other than 0, then
fiemap will not be allocated. However,
GenericFileStoreBackend::detect_features will free fiemap regardless,
triggering an assertion in tcmalloc. The attached patch will fix the bug
by only freeing fiemap when necessary.

In my case, do_fiemap returns -ENOSYS because fiemap is not implemented
on FreeBSD. However, this bug could also happen on Linux when do_fiemap
returns -ENOMEM.

Fixes: #6504
Signed-off-by: Alan Somers <asomers@gmail.com>
Reviewed-by: Sage Weil <sage@inktank.com>

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 20afde9a0dcec85ec058cc401a860bf6359b10bc..6940dff140569252840dbf395960c4e9222e3538 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -2587,8 +2587,10 @@ int FileStore::fiemap(coll_t cid, const ghobject_t& oid,
     if (r < 0)
       goto done;
 
-    if (fiemap->fm_mapped_extents == 0)
+    if (fiemap->fm_mapped_extents == 0) {
+      free(fiemap);
       goto done;
+    }
 
     struct fiemap_extent *extent = &fiemap->fm_extents[0];
 
@@ -2622,6 +2624,7 @@ int FileStore::fiemap(coll_t cid, const ghobject_t& oid,
       i++;
       extent++;
     }
+    free(fiemap);
   }
 
 done:
@@ -2631,7 +2634,6 @@ done:
   }
 
   dout(10) << "fiemap " << cid << "/" << oid << " " << offset << "~" << len << " = " << r << " num_extents=" << exomap.size() << " " << exomap << dendl;
-  free(fiemap);
   assert(!m_filestore_fail_eio || r != -EIO);
   return r;
 }

diff --git a/src/os/GenericFileStoreBackend.cc b/src/os/GenericFileStoreBackend.cc
index 81d896a09435b61f640fc57d3d3ba21bf38c912f..f19ba7d7760237c54bd80fdd2f65994fcbc6ad1e 100644 (file)
--- a/src/os/GenericFileStoreBackend.cc
+++ b/src/os/GenericFileStoreBackend.cc
@@ -124,12 +124,12 @@ int GenericFileStoreBackend::detect_features()
       dout(0) << "detect_features: FIEMAP ioctl is supported and appears to work" << dendl;
       ioctl_fiemap = true;
     }
+    free(fiemap);
   }
   if (!m_filestore_fiemap) {
     dout(0) << "detect_features: FIEMAP ioctl is disabled via 'filestore fiemap' config option" << dendl;
     ioctl_fiemap = false;
   }
-  free(fiemap);
 
   ::unlink(fn);
   TEMP_FAILURE_RETRY(::close(fd));