[PATCH 0/2] fix null deref in readdir_r callback and test

[PATCH 0/2] fix null deref in readdir_r callback and test

[Brian Chrisman]

_readdir_single_dirent_cb is invoked with zeroed pointers when called beneath readdir_r rather than directly readdirplus_r.
Those pointers are then dereferenced in assignment.
There is still a problem in readdir_r, so I extended the basic scenario in testceph.cc.
Methods readdir_r and readdirplus_r are bypassed in cfuse.

Brian Chrisman (2):
  fix null deref when callback invoked en route from readdir_r rather
    than readdirplus_r
  add basic test case for readdir_r

 src/client/Client.cc   |    8 ++++++--
 src/client/testceph.cc |   36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

[PATCH 1/2] fix null deref when callback invoked en route from readdir_r rather than readdirplus_r

[Brian Chrisman]

Signed-off-by: Brian Chrisman <brchrisman@gmail.com>
---
 src/client/Client.cc |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 8b8fcf1..dbac2c9 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -4330,8 +4330,12 @@ static int _readdir_single_dirent_cb(void *p, struct dirent *de, struct stat *st
     return -1;
 
   *c->de = *de;
-  *c->st = *st;
-  *c->stmask = stmask;
+  if (c->st) {
+    *c->st = *st;
+  }
+  if (c->stmask) {
+    *c->stmask = stmask;
+  }
   c->full = true;
   return 0;  
 }
-- 
1.7.1

[PATCH 2/2] add basic test case for readdir_r

[Brian Chrisman]

Signed-off-by: Brian Chrisman <brchrisman@gmail.com>
---
 src/client/testceph.cc |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/src/client/testceph.cc b/src/client/testceph.cc
index 520fa75..c24cc03 100644
--- a/src/client/testceph.cc
+++ b/src/client/testceph.cc
@@ -17,6 +17,7 @@
 #include <stdlib.h>
 
 #include <errno.h>
+#include <dirent.h>
 #include <iostream>
 #include <fcntl.h>
 #include <sys/xattr.h>
@@ -190,8 +191,41 @@ int main(int argc, const char **argv)
   } else {
     cout << "ceph_lstat: success" << std::endl;
   }
+  cout << "Attempting readdir_r" << std::endl;
+  ret = ceph_mkdir(cmount, "readdir_r_test",  0777);
+  if (ret) {
+    cerr << "ceph_mkdir error: " << cpp_strerror(ret) << std::endl;
+    return 1;
+  } else {
+    cout << "ceph_mkdir: success" << std::endl;
+  }
+  struct ceph_dir_result *readdir_r_test_dir;
+  ret = ceph_opendir(cmount, "readdir_r_test", &readdir_r_test_dir);
+  if (ret != 0) {
+    cerr << "ceph_opendir error: unexpected result from trying to open readdir_r_test: "
+	 << cpp_strerror(ret) << std::endl;
+    return 1;
+  } else {
+    cout << "ceph_opendir: success" << std::endl;
+  }
+  ret = ceph_open(cmount, "readdir_r_test/opened_file", O_CREAT, 0666);
+  if (ret < 0) {
+    cerr << "ceph_open O_CREAT error: " << cpp_strerror(ret) << std::endl;
+    return 1;
+  } else {
+    cout << "ceph_open: success" << std::endl;
+  }
 
-
+  struct dirent * result;
+  result = (struct dirent *) malloc(sizeof(struct dirent));
+  ret = ceph_readdir_r(cmount, readdir_r_test_dir, result);
+  if (ret != 0) {
+    cerr << "ceph_readdir_r: fail, returned: " << ret << std::endl;
+  } else {
+    cerr << "ceph_readdir_r: success: " << *result->d_name << std::endl;
+    return 1;
+  }
+  
   ceph_shutdown(cmount);
 
   return 0;
-- 
1.7.1