From: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
Date: Thu, 29 Apr 2021 10:57:35 +0000 (+0900)
Subject: os/FileStore: fix to handle readdir error correctly
X-Git-Tag: v15.2.13~15^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F41237%2Fhead;p=ceph.git

os/FileStore: fix to handle readdir error correctly

Currently filestore code does not handle readdir error.
As man readdir(3) says, we need to check errno after readdir
returns NULL to determine if error happens or not.

This patch fixes the all readdir() calls to check errono and
handle it appropriately:
 - FileStore.cc ... abort if EIO error happens
 - BtrfsFileStoreBAckend.cc/LFNindex.cc
                ... return error to upper layer

Without this fixes, primary PG could fail to correctly perform
backfill operation and could lead data loss propagation described
in #50558.

Fixes: https://tracker.ceph.com/issues/50558
Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
(cherry picked from commit 5a6c6267a182f859471ee629b490777ee1e970dd)
---

diff --git a/src/os/filestore/BtrfsFileStoreBackend.cc b/src/os/filestore/BtrfsFileStoreBackend.cc
index d2d016e790fe..e95f2d47d414 100644
--- a/src/os/filestore/BtrfsFileStoreBackend.cc
+++ b/src/os/filestore/BtrfsFileStoreBackend.cc
@@ -322,7 +322,17 @@ int BtrfsFileStoreBackend::list_checkpoints(list<string>& ls)
   list<string> snaps;
   char path[PATH_MAX];
   struct dirent *de;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        err = -errno;
+        dout(0) << "list_checkpoints: readdir '" << get_basedir_path() << "' failed: "
+                << cpp_strerror(err) << dendl;
+      }
+      break;
+    }
     snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(), de->d_name);
 
     struct stat st;
diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index d6aceafda176..487298575a03 100644
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -4966,7 +4966,17 @@ int FileStore::list_collections(vector<coll_t>& ls, bool include_temp)
   }
 
   struct dirent *de = nullptr;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        derr << "readdir failed " << fn << ": " << cpp_strerror(-r) << dendl;
+        if (r == -EIO && m_filestore_fail_eio) handle_eio();
+      }
+      break;
+    }
     if (de->d_type == DT_UNKNOWN) {
       // d_type not supported (non-ext[234], btrfs), must stat
       struct stat sb;
diff --git a/src/os/filestore/LFNIndex.cc b/src/os/filestore/LFNIndex.cc
index 2451ae8c7052..2aacdceffdc4 100644
--- a/src/os/filestore/LFNIndex.cc
+++ b/src/os/filestore/LFNIndex.cc
@@ -420,7 +420,18 @@ int LFNIndex::list_objects(const vector<string> &to_list, int max_objs,
   int r = 0;
   int listed = 0;
   bool end = true;
-  while ((de = ::readdir(dir))) {
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        dout(0) << "readdir failed " << to_list_path << ": "
+                << cpp_strerror(-r) << dendl;
+        goto cleanup;
+      }
+      break;
+    }
     end = false;
     if (max_objs > 0 && listed >= max_objs) {
       break;
@@ -468,7 +479,18 @@ int LFNIndex::list_subdirs(const vector<string> &to_list,
     return -errno;
 
   struct dirent *de = nullptr;
-  while ((de = ::readdir(dir))) {
+  int r = 0;
+  while (true) {
+    errno = 0;
+    de = ::readdir(dir);
+    if (de == nullptr) {
+      if (errno != 0) {
+        r = -errno;
+        dout(0) << "readdir failed " << to_list_path << ": "
+                << cpp_strerror(-r) << dendl;
+      }
+      break;
+    }
     string short_name(de->d_name);
     string demangled_name;
     if (lfn_is_subdir(short_name, &demangled_name)) {
@@ -477,7 +499,7 @@ int LFNIndex::list_subdirs(const vector<string> &to_list,
   }
 
   ::closedir(dir);
-  return 0;
+  return r;
 }
 
 int LFNIndex::create_path(const vector<string> &to_create)