Merge PR ceph#55457 into main

* refs/pull/55457/head:
	client: check for bad file handle in low level I/O APIs
	client: check for bad file handle in ll_preadv_pwritev
	client: add function to check if file handle exists
	src/test: test async I/O with invalid/closed file handle

Reviewed-by: Venky Shankar <[email protected]>

Author: vshankar

src/client/Client.cc

@@ -15779,8 +15779,14 @@ loff_t Client::ll_lseek(Fh *fh, loff_t offset, int whence)
 int Client::ll_read(Fh *fh, loff_t off, loff_t len, bufferlist *bl)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
-  if (!mref_reader.is_state_satisfied())
+  if (!mref_reader.is_state_satisfied()) {
     return -CEPHFS_ENOTCONN;
+  }
+
+  if (fh == NULL || !_ll_fh_exists(fh)) {
+    ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
+    return -CEPHFS_EBADF;
+  }
 
   ldout(cct, 3) << "ll_read " << fh << " " << fh->inode->ino << " " << " " << off << "~" << len << dendl;
   tout(cct) << "ll_read" << std::endl;
@@ -15917,17 +15923,23 @@ int Client::ll_commit_blocks(Inode *in,
 
 int Client::ll_write(Fh *fh, loff_t off, loff_t len, const char *data)
 {
+  RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
+  if (!mref_reader.is_state_satisfied()) {
+    return -CEPHFS_ENOTCONN;
+  }
+
+  if (fh == NULL || !_ll_fh_exists(fh)) {
+    ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
+    return -CEPHFS_EBADF;
+  }
+
   ldout(cct, 3) << "ll_write " << fh << " " << fh->inode->ino << " " << off <<
     "~" << len << dendl;
   tout(cct) << "ll_write" << std::endl;
   tout(cct) << (uintptr_t)fh << std::endl;
   tout(cct) << off << std::endl;
   tout(cct) << len << std::endl;
 
-  RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
-  if (!mref_reader.is_state_satisfied())
-    return -CEPHFS_ENOTCONN;
-
   /* We can't return bytes written larger than INT_MAX, clamp len to that */
   len = std::min(len, (loff_t)INT_MAX);
   std::scoped_lock lock(client_lock);
@@ -15941,8 +15953,14 @@ int Client::ll_write(Fh *fh, loff_t off, loff_t len, const char *data)
 int64_t Client::ll_writev(struct Fh *fh, const struct iovec *iov, int iovcnt, int64_t off)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
-  if (!mref_reader.is_state_satisfied())
+  if (!mref_reader.is_state_satisfied()) {
     return -CEPHFS_ENOTCONN;
+  }
+
+  if (fh == NULL || !_ll_fh_exists(fh)) {
+    ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
+    return -CEPHFS_EBADF;
+  }
 
   std::scoped_lock cl(client_lock);
   return _preadv_pwritev_locked(fh, iov, iovcnt, off, true, false);
@@ -15951,8 +15969,14 @@ int64_t Client::ll_writev(struct Fh *fh, const struct iovec *iov, int iovcnt, in
 int64_t Client::ll_readv(struct Fh *fh, const struct iovec *iov, int iovcnt, int64_t off)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
-  if (!mref_reader.is_state_satisfied())
+  if (!mref_reader.is_state_satisfied()) {
     return -CEPHFS_ENOTCONN;
+  }
+
+  if (fh == NULL || !_ll_fh_exists(fh)) {
+    ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
+    return -CEPHFS_EBADF;
+  }
 
   std::scoped_lock cl(client_lock);
   return _preadv_pwritev_locked(fh, iov, iovcnt, off, false, false);
@@ -15963,23 +15987,34 @@ int64_t Client::ll_preadv_pwritev(struct Fh *fh, const struct iovec *iov,
                                   Context *onfinish, bufferlist *bl,
                                   bool do_fsync, bool syncdataonly)
 {
+    int64_t retval = -1;
+
     RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
     if (!mref_reader.is_state_satisfied()) {
-      int64_t rc = -CEPHFS_ENOTCONN;
+      retval = -CEPHFS_ENOTCONN;
       if (onfinish != nullptr) {
-        onfinish->complete(rc);
+        onfinish->complete(retval);
         /* async call should always return zero to caller and allow the
         caller to wait on callback for the actual errno. */
-        rc = 0;
+        retval = 0;
+      }
+      return retval;
+    }
+
+    if(fh == NULL || !_ll_fh_exists(fh)) {
+      ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
+      retval = -CEPHFS_EBADF;
+      if (onfinish != nullptr) {
+        onfinish->complete(retval);
+        retval = 0;
       }
-      return rc;
+      return retval;
     }
 
     std::scoped_lock cl(client_lock);
 
-    int64_t retval = _preadv_pwritev_locked(fh, iov, iovcnt, offset, write,
-                                            true, onfinish, bl, do_fsync,
-                                            syncdataonly);
+    retval = _preadv_pwritev_locked(fh, iov, iovcnt, offset, write, true,
+                                    onfinish, bl, do_fsync, syncdataonly);
     /* There are two scenarios with each having two cases to handle here
     1) async io
       1.a) r == 0:

src/client/Client.h

@@ -1028,6 +1028,9 @@ class Client : public Dispatcher, public md_config_obs_t {
     return it->second;
   }
   int get_fd_inode(int fd, InodeRef *in);
+  bool _ll_fh_exists(Fh *f) {
+    return ll_unclosed_fh_set.count(f);
+  }
 
   // helpers
   void wake_up_session_caps(MetaSession *s, bool reconnect);

src/test/client/nonblocking.cc

@@ -526,4 +526,88 @@ TEST_F(TestClient, LlreadvLlwritevZeroBytes) {
 
   client->ll_release(fh);
   ASSERT_EQ(0, client->ll_unlink(root, filename, myperm));
-}
+}
+
+TEST_F(TestClient, LlreadvLlwritevInvalidFileHandle) {
+  /* Test provding null or invalid file handle returns an error
+  as expected*/
+
+  Fh *fh_null = NULL;
+
+  char out_buf_0[] = "hello ";
+  char out_buf_1[] = "world\n";
+  struct iovec iov_out[2] = {
+    {out_buf_0, sizeof(out_buf_0)},
+    {out_buf_1, sizeof(out_buf_1)},
+  };
+
+  char in_buf_0[sizeof(out_buf_0)];
+  char in_buf_1[sizeof(out_buf_1)];
+  struct iovec iov_in[2] = {
+    {in_buf_0, sizeof(in_buf_0)},
+    {in_buf_1, sizeof(in_buf_1)},
+  };
+
+  std::unique_ptr<C_SaferCond> writefinish = nullptr;
+  std::unique_ptr<C_SaferCond> readfinish = nullptr;
+
+  writefinish.reset(new C_SaferCond("test-nonblocking-writefinish-null-fh"));
+  readfinish.reset(new C_SaferCond("test-nonblocking-readfinish-null-fh"));
+
+  int64_t rc;
+  bufferlist bl;
+  ssize_t bytes_written = 0, bytes_read = 0;
+
+  rc = client->ll_preadv_pwritev(fh_null, iov_out, 2, 0, true,
+                                 writefinish.get(), nullptr);
+  ASSERT_EQ(rc, 0);
+  bytes_written = writefinish->wait();
+  ASSERT_EQ(bytes_written, -CEPHFS_EBADF);
+
+  rc = client->ll_preadv_pwritev(fh_null, iov_in, 2, 0, false,
+                                 readfinish.get(), &bl);
+  ASSERT_EQ(rc, 0);
+  bytes_read = readfinish->wait();
+  ASSERT_EQ(bytes_read, -CEPHFS_EBADF);
+  ASSERT_EQ(bl.length(), 0);
+
+  // test after closing the file handle
+  int mypid = getpid();
+  char filename[256];
+
+  client->unmount();
+  TearDown();
+  SetUp();
+
+  sprintf(filename, "test_llreadvllwritevinvalidfhfile%u", mypid);
+
+  Inode *root, *file;
+  root = client->get_root();
+  ASSERT_NE(root, (Inode *)NULL);
+
+  Fh *fh;
+  struct ceph_statx stx;
+
+  ASSERT_EQ(0, client->ll_createx(root, filename, 0666,
+				  O_RDWR | O_CREAT | O_TRUNC,
+				  &file, &fh, &stx, 0, 0, myperm));
+
+  client->ll_release(fh);
+  ASSERT_EQ(0, client->ll_unlink(root, filename, myperm));
+
+  writefinish.reset(new C_SaferCond("test-nonblocking-writefinish-invalid-fh"));
+  readfinish.reset(new C_SaferCond("test-nonblocking-readfinish-invalid-fh"));
+
+  rc = client->ll_preadv_pwritev(fh, iov_out, 2, 0, true, writefinish.get(),
+                                 nullptr);
+  ASSERT_EQ(rc, 0);
+  bytes_written = writefinish->wait();
+  ASSERT_EQ(bytes_written, -CEPHFS_EBADF);
+
+  rc = client->ll_preadv_pwritev(fh, iov_in, 2, 0, false, readfinish.get(),
+                                 &bl);
+  ASSERT_EQ(rc, 0);
+  bytes_read = readfinish->wait();
+  ASSERT_EQ(bytes_read, -CEPHFS_EBADF);
+  ASSERT_EQ(bl.length(), 0);
+}