From: Michal Jarzabek <stiopa@gmail.com>
Date: Thu, 12 Jan 2017 21:22:20 +0000 (+0000)
Subject: client/Client.cc: prevent segfaulting
X-Git-Tag: v10.2.6~122^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F13029%2Fhead;p=ceph.git

client/Client.cc: prevent segfaulting

The segfaulting in the rmdir function is caused by calling
filepath::last_dentry() function.
last_dentry() function assumes that the bits vector has always at
least one element, which is not the case for the the filepath object
created with "/" input.
This commit also fixes other functions affected by this bug:
link, unlink, rename, mkdir, mknod and symlink.

Fixes: http://tracker.ceph.com/issues/9935
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
(cherry picked from commit 6ed7f2364ae5507bab14c60b582929aa7b0ba400)

Conflicts:
	src/client/Client.cc (Client.cc - path_walk(), may_create(), and
                              _link() take fewer parameters in jewel)
	src/test/libcephfs/test.cc (preceding tests are missing in jewel)
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 282c57bb1816..c9a63998a5c7 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -6091,31 +6091,35 @@ int Client::link(const char *relexisting, const char *relpath)
   tout(cct) << relpath << std::endl;
 
   filepath existing(relexisting);
-  filepath path(relpath);
-  string name = path.last_dentry();
-  path.pop_dentry();
 
   InodeRef in, dir;
   int r = path_walk(existing, &in);
   if (r < 0)
-    goto out;
+    return r;
+  if (std::string(relpath) == "/") {
+    r = -EEXIST;
+    return r;
+  }
+  filepath path(relpath);
+  string name = path.last_dentry();
+  path.pop_dentry();
+
   r = path_walk(path, &dir);
   if (r < 0)
-    goto out;
+    return r;
   if (cct->_conf->client_permissions) {
     if (S_ISDIR(in->mode)) {
       r = -EPERM;
-      goto out;
+      return r;
     }
     r = may_hardlink(in.get());
     if (r < 0)
-      goto out;
+      return r;
     r = may_create(dir.get());
     if (r < 0)
-      goto out;
+      return r;
   }
   r = _link(in.get(), dir.get(), name.c_str());
- out:
   return r;
 }
 
@@ -6125,6 +6129,9 @@ int Client::unlink(const char *relpath)
   tout(cct) << "unlink" << std::endl;
   tout(cct) << relpath << std::endl;
 
+  if (std::string(relpath) == "/")
+    return -EISDIR;
+
   filepath path(relpath);
   string name = path.last_dentry();
   path.pop_dentry();
@@ -6147,6 +6154,9 @@ int Client::rename(const char *relfrom, const char *relto)
   tout(cct) << relfrom << std::endl;
   tout(cct) << relto << std::endl;
 
+  if (std::string(relfrom) == "/" || std::string(relto) == "/")
+    return -EBUSY;
+
   filepath from(relfrom);
   filepath to(relto);
   string fromname = from.last_dentry();
@@ -6185,6 +6195,9 @@ int Client::mkdir(const char *relpath, mode_t mode)
   tout(cct) << mode << std::endl;
   ldout(cct, 10) << "mkdir: " << relpath << dendl;
 
+  if (std::string(relpath) == "/")
+    return -EEXIST;
+
   filepath path(relpath);
   string name = path.last_dentry();
   path.pop_dentry();
@@ -6257,6 +6270,10 @@ int Client::rmdir(const char *relpath)
   Mutex::Locker lock(client_lock);
   tout(cct) << "rmdir" << std::endl;
   tout(cct) << relpath << std::endl;
+
+  if (std::string(relpath) == "/")
+    return -EBUSY;
+
   filepath path(relpath);
   string name = path.last_dentry();
   path.pop_dentry();
@@ -6279,6 +6296,10 @@ int Client::mknod(const char *relpath, mode_t mode, dev_t rdev)
   tout(cct) << relpath << std::endl;
   tout(cct) << mode << std::endl;
   tout(cct) << rdev << std::endl;
+
+  if (std::string(relpath) == "/")
+    return -EEXIST;
+
   filepath path(relpath);
   string name = path.last_dentry();
   path.pop_dentry();
@@ -6303,6 +6324,9 @@ int Client::symlink(const char *target, const char *relpath)
   tout(cct) << target << std::endl;
   tout(cct) << relpath << std::endl;
 
+  if (std::string(relpath) == "/")
+    return -EEXIST;
+
   filepath path(relpath);
   string name = path.last_dentry();
   path.pop_dentry();
diff --git a/src/include/filepath.h b/src/include/filepath.h
index 15c3783f03af..b4c33070c9ff 100644
--- a/src/include/filepath.h
+++ b/src/include/filepath.h
@@ -130,6 +130,7 @@ class filepath {
 
   const string& last_dentry() const {
     if (bits.empty() && path.length() > 0) parse_bits();
+    assert(!bits.empty());
     return bits[ bits.size()-1 ];
   }
 
diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc
index b0314b618524..42b02f11d7bf 100644
--- a/src/test/libcephfs/test.cc
+++ b/src/test/libcephfs/test.cc
@@ -1374,3 +1374,41 @@ TEST(LibCephFS, OpenNoClose) {
   // shutdown should force close opened file/dir
   ceph_shutdown(cmount);
 }
+
+TEST(LibCephFS, OperationsOnRoot)
+{
+  struct ceph_mount_info *cmount;
+  ASSERT_EQ(ceph_create(&cmount, NULL), 0);
+  ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0);
+  ASSERT_EQ(0, ceph_conf_parse_env(cmount, NULL));
+  ASSERT_EQ(ceph_mount(cmount, "/"), 0);
+
+  char dirname[32];
+  sprintf(dirname, "/somedir%x", getpid());
+
+  ASSERT_EQ(ceph_mkdir(cmount, dirname, 0755), 0);
+
+  ASSERT_EQ(ceph_rmdir(cmount, "/"), -EBUSY);
+
+  ASSERT_EQ(ceph_link(cmount, "/", "/"), -EEXIST);
+  ASSERT_EQ(ceph_link(cmount, dirname, "/"), -EEXIST);
+  ASSERT_EQ(ceph_link(cmount, "nonExisitingDir", "/"), -ENOENT);
+
+  ASSERT_EQ(ceph_unlink(cmount, "/"), -EISDIR);
+
+  ASSERT_EQ(ceph_rename(cmount, "/", "/"), -EBUSY);
+  ASSERT_EQ(ceph_rename(cmount, dirname, "/"), -EBUSY);
+  ASSERT_EQ(ceph_rename(cmount, "nonExistingDir", "/"), -EBUSY);
+  ASSERT_EQ(ceph_rename(cmount, "/", dirname), -EBUSY);
+  ASSERT_EQ(ceph_rename(cmount, "/", "nonExistingDir"), -EBUSY);
+
+  ASSERT_EQ(ceph_mkdir(cmount, "/", 0777), -EEXIST);
+
+  ASSERT_EQ(ceph_mknod(cmount, "/", 0, 0), -EEXIST);
+
+  ASSERT_EQ(ceph_symlink(cmount, "/", "/"), -EEXIST);
+  ASSERT_EQ(ceph_symlink(cmount, dirname, "/"), -EEXIST);
+  ASSERT_EQ(ceph_symlink(cmount, "nonExistingDir", "/"), -EEXIST);
+
+  ceph_shutdown(cmount);
+}