client: return EBADF for invalid file desc

Adds get_filehandle to Client which resolves a file descriptor or
returns NULL if the file descriptor is invalid. Libcephfs calls that
accept a file descriptor are changed to return -EBADF when
get_filehandle returns NULL.

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Reviewed-by: Sage Weil <sage@inktank.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index fda5b21f2114e3bfc6dfe0641d116565c6832b0c..7efbfcc1f924cecdd334baa93a6b0418628a5b85 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -4229,8 +4229,9 @@ int Client::fchmod(int fd, mode_t mode)
   tout(cct) << "fchmod" << std::endl;
   tout(cct) << fd << std::endl;
   tout(cct) << mode << std::endl;
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   struct stat attr;
   attr.st_mode = mode;
   return _setattr(f->inode, &attr, CEPH_SETATTR_MODE);
@@ -5118,8 +5119,9 @@ int Client::close(int fd)
   tout(cct) << "close" << std::endl;
   tout(cct) << fd << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *fh = fd_map[fd];
+  Fh *fh = get_filehandle(fd);
+  if (!fh)
+    return -EBADF;
   _release_fh(fh);
   fd_map.erase(fd);
   ldout(cct, 3) << "close exit(" << fd << ")" << dendl;
@@ -5138,8 +5140,9 @@ loff_t Client::lseek(int fd, loff_t offset, int whence)
   tout(cct) << offset << std::endl;
   tout(cct) << whence << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   return _lseek(f, offset, whence);
 }
 
@@ -5210,8 +5213,9 @@ int Client::read(int fd, char *buf, loff_t size, loff_t offset)
   tout(cct) << size << std::endl;
   tout(cct) << offset << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   bufferlist bl;
   int r = _read(f, offset, size, &bl);
   ldout(cct, 3) << "read(" << fd << ", " << (void*)buf << ", " << size << ", " << offset << ") = " << r << dendl;
@@ -5463,8 +5467,9 @@ int Client::write(int fd, const char *buf, loff_t size, loff_t offset)
   tout(cct) << size << std::endl;
   tout(cct) << offset << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *fh = fd_map[fd];
+  Fh *fh = get_filehandle(fd);
+  if (!fh)
+    return -EBADF;
   int r = _write(fh, offset, size, buf);
   ldout(cct, 3) << "write(" << fd << ", \"...\", " << size << ", " << offset << ") = " << r << dendl;
   return r;
@@ -5612,8 +5617,9 @@ int Client::ftruncate(int fd, loff_t length)
   tout(cct) << fd << std::endl;
   tout(cct) << length << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   struct stat attr;
   attr.st_size = length;
   return _setattr(f->inode, &attr, CEPH_SETATTR_SIZE);
@@ -5626,8 +5632,9 @@ int Client::fsync(int fd, bool syncdataonly)
   tout(cct) << fd << std::endl;
   tout(cct) << syncdataonly << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   int r = _fsync(f, syncdataonly);
   ldout(cct, 3) << "fsync(" << fd << ", " << syncdataonly << ") = " << r << dendl;
   return r;
@@ -5677,8 +5684,9 @@ int Client::fstat(int fd, struct stat *stbuf)
   tout(cct) << "fstat" << std::endl;
   tout(cct) << fd << std::endl;
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   int r = _getattr(f->inode, -1);
   if (r < 0)
     return r;
@@ -5841,8 +5849,9 @@ int Client::lazyio_propogate(int fd, loff_t offset, size_t count)
   ldout(cct, 3) << "op: client->lazyio_propogate(" << fd
           << ", " << offset << ", " << count << ")" << dendl;
   
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
 
   // for now
   _fsync(f, true);
@@ -5857,8 +5866,9 @@ int Client::lazyio_synchronize(int fd, loff_t offset, size_t count)
   ldout(cct, 3) << "op: client->lazyio_synchronize(" << fd
           << ", " << offset << ", " << count << ")" << dendl;
   
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   Inode *in = f->inode;
   
   _fsync(f, true);
@@ -7045,8 +7055,9 @@ int Client::describe_layout(int fd, ceph_file_layout *lp)
 {
   Mutex::Locker lock(client_lock);
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   Inode *in = f->inode;
 
   *lp = in->layout;
@@ -7070,8 +7081,9 @@ int Client::get_file_stripe_address(int fd, loff_t offset, vector<entity_addr_t>
 {
   Mutex::Locker lock(client_lock);
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   Inode *in = f->inode;
 
   // which object?
@@ -7099,8 +7111,9 @@ int Client::enumerate_layout(int fd, vector<ObjectExtent>& result,
 {
   Mutex::Locker lock(client_lock);
 
-  assert(fd_map.count(fd));
-  Fh *f = fd_map[fd];
+  Fh *f = get_filehandle(fd);
+  if (!f)
+    return -EBADF;
   Inode *in = f->inode;
 
   // map to a list of extents

diff --git a/src/client/Client.h b/src/client/Client.h
index 8edf9f550f6a91443afdec61d975b2cd924b8cca..b086662b1f9f917abb91556ed6e1c0bd76810ff3 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -310,6 +310,16 @@ protected:
     free_fd_set.insert(fd, 1);
   }
 
+  /*
+   * Resolve file descriptor, or return NULL.
+   */
+  Fh *get_filehandle(int fd) {
+    hash_map<int, Fh*>::iterator p = fd_map.find(fd);
+    if (p == fd_map.end())
+      return NULL;
+    return p->second;
+  }
+
   // global client lock
   //  - protects Client and buffer cache both!
   Mutex                  client_lock;

diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc
index 1d4b45e15a7ae2fc2a964b05030b2d2b0d6bf641..30dbc0265576fb8251dc21ecf02cede50ae94b09 100644 (file)
--- a/src/test/libcephfs/test.cc
+++ b/src/test/libcephfs/test.cc
@@ -616,4 +616,30 @@ TEST(LibCephFS, Hardlink_no_original) {
   ceph_shutdown(cmount);
 }
 
+TEST(LibCephFS, BadFileDesc) {
+  struct ceph_mount_info *cmount;
+  ASSERT_EQ(ceph_create(&cmount, NULL), 0);
+  ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0);
+  ASSERT_EQ(ceph_mount(cmount, NULL), 0);
+
+  ASSERT_EQ(ceph_fchmod(cmount, -1, 0655), -EBADF);
+  ASSERT_EQ(ceph_close(cmount, -1), -EBADF);
+  ASSERT_EQ(ceph_lseek(cmount, -1, 0, SEEK_SET), -EBADF);
 
+  char buf[0];
+  ASSERT_EQ(ceph_read(cmount, -1, buf, 0, 0), -EBADF);
+  ASSERT_EQ(ceph_write(cmount, -1, buf, 0, 0), -EBADF);
+
+  ASSERT_EQ(ceph_ftruncate(cmount, -1, 0), -EBADF);
+  ASSERT_EQ(ceph_fsync(cmount, -1, 0), -EBADF);
+
+  struct stat stat;
+  ASSERT_EQ(ceph_fstat(cmount, -1, &stat), -EBADF);
+
+  struct sockaddr_storage addr;
+  ASSERT_EQ(ceph_get_file_stripe_address(cmount, -1, 0, &addr, 1), -EBADF);
+
+  ASSERT_EQ(ceph_get_file_stripe_unit(cmount, -1), -EBADF);
+  ASSERT_EQ(ceph_get_file_pool(cmount, -1), -EBADF);
+  ASSERT_EQ(ceph_get_file_replication(cmount, -1), -EBADF);
+}