Merge PR ceph#58564 into main

* refs/pull/58564/head:
	client: clamp sizes to INT_MAX in sync i/o code paths
	client: restrict bufferlist to total write size
	src/test: test sync/async i/o code paths with huge (4GiB) buffers

Reviewed-by: Venky Shankar <[email protected]>
Reviewed-by: Kotresh Hiremath Ravishankar <[email protected]>
Reviewed-by: Christopher Hoffman <[email protected]>

Author: vshankar

src/client/Client.cc

@@ -11474,7 +11474,9 @@ int Client::write(int fd, const char *buf, loff_t size, loff_t offset)
 #endif
   /* We can't return bytes written larger than INT_MAX, clamp size to that */
   size = std::min(size, (loff_t)INT_MAX);
-  int r = _write(fh, offset, size, buf, NULL, false);
+  bufferlist bl;
+  bl.append(buf, size);
+  int r = _write(fh, offset, size, std::move(bl));
   ldout(cct, 3) << "write(" << fd << ", \"...\", " << size << ", " << offset << ") = " << r << dendl;
   return r;
 }
@@ -11499,7 +11501,7 @@ int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
     if(iovcnt < 0) {
       return -EINVAL;
     }
-    loff_t totallen = 0;
+    size_t totallen = 0;
     for (int i = 0; i < iovcnt; i++) {
         totallen += iov[i].iov_len;
     }
@@ -11509,12 +11511,29 @@ int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
      * 32-bit signed integers. Clamp the I/O sizes in those functions so that
      * we don't do I/Os larger than the values we can return.
      */
+    bufferlist data;
     if (clamp_to_int) {
-      totallen = std::min(totallen, (loff_t)INT_MAX);
+      totallen = std::min(totallen, (size_t)INT_MAX);
+      size_t total_appended = 0;
+      for (int i = 0; i < iovcnt; i++) {
+        if (iov[i].iov_len > 0) {
+          if (total_appended + iov[i].iov_len >= totallen) {
+            data.append((const char *)iov[i].iov_base, totallen - total_appended);
+            break;
+          } else {
+            data.append((const char *)iov[i].iov_base, iov[i].iov_len);
+            total_appended += iov[i].iov_len;
+          }
+        }
+      }
+    } else {
+      for (int i = 0; i < iovcnt; i++) {
+        data.append((const char *)iov[i].iov_base, iov[i].iov_len);
+      }
     }
 
     if (write) {
-        int64_t w = _write(fh, offset, totallen, NULL, iov, iovcnt, onfinish, do_fsync, syncdataonly);
+        int64_t w = _write(fh, offset, totallen, std::move(data), onfinish, do_fsync, syncdataonly);
         ldout(cct, 3) << "pwritev(" << fh << ", \"...\", " << totallen << ", " << offset << ") = " << w << dendl;
         return w;
     } else {
@@ -11698,9 +11717,8 @@ bool Client::C_Write_Finisher::try_complete()
   return false;
 }
 
-int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
-	                const struct iovec *iov, int iovcnt, Context *onfinish,
-	                bool do_fsync, bool syncdataonly)
+int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, bufferlist bl,
+	               Context *onfinish, bool do_fsync, bool syncdataonly)
 {
   ceph_assert(ceph_mutex_is_locked_by_me(client_lock));
 
@@ -11770,19 +11788,6 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
     ceph_assert(in->inline_version > 0);
   }
 
-  // copy into fresh buffer (since our write may be resub, async)
-  bufferlist bl;
-  if (buf) {
-    if (size > 0)
-      bl.append(buf, size);
-  } else if (iov){
-    for (int i = 0; i < iovcnt; i++) {
-      if (iov[i].iov_len > 0) {
-        bl.append((const char *)iov[i].iov_base, iov[i].iov_len);
-      }
-    }
-  }
-
   int want, have;
   if (f->mode & CEPH_FILE_MODE_LAZY)
     want = CEPH_CAP_FILE_BUFFER | CEPH_CAP_FILE_LAZYIO;
@@ -16149,7 +16154,9 @@ int Client::ll_write(Fh *fh, loff_t off, loff_t len, const char *data)
   tout(cct) << off << std::endl;
   tout(cct) << len << std::endl;
 
-  int r = _write(fh, off, len, data, NULL, 0);
+  bufferlist bl;
+  bl.append(data, len);
+  int r = _write(fh, off, len, std::move(bl));
   ldout(cct, 3) << "ll_write " << fh << " " << off << "~" << len << " = " << r
 		<< dendl;
   return r;
@@ -16167,7 +16174,7 @@ int64_t Client::ll_writev(struct Fh *fh, const struct iovec *iov, int iovcnt, in
     ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
     return -EBADF;
   }
-  return _preadv_pwritev_locked(fh, iov, iovcnt, off, true, false);
+  return _preadv_pwritev_locked(fh, iov, iovcnt, off, true, true);
 }
 
 int64_t Client::ll_readv(struct Fh *fh, const struct iovec *iov, int iovcnt, int64_t off)
@@ -16182,7 +16189,7 @@ int64_t Client::ll_readv(struct Fh *fh, const struct iovec *iov, int iovcnt, int
     ldout(cct, 3) << "(fh)" << fh << " is invalid" << dendl;
     return -EBADF;
   }
-  return _preadv_pwritev_locked(fh, iov, iovcnt, off, false, false);
+  return _preadv_pwritev_locked(fh, iov, iovcnt, off, false, true);
 }
 
 int64_t Client::ll_preadv_pwritev(struct Fh *fh, const struct iovec *iov,

src/client/Client.h

@@ -1758,9 +1758,9 @@ class Client : public Dispatcher, public md_config_obs_t {
   void do_readahead(Fh *f, Inode *in, uint64_t off, uint64_t len);
   int64_t _write_success(Fh *fh, utime_t start, uint64_t fpos,
           int64_t offset, uint64_t size, Inode *in);
-  int64_t _write(Fh *fh, int64_t offset, uint64_t size, const char *buf,
-          const struct iovec *iov, int iovcnt, Context *onfinish = nullptr,
-          bool do_fsync = false, bool syncdataonly = false);
+  int64_t _write(Fh *fh, int64_t offset, uint64_t size, bufferlist bl,
+          Context *onfinish = nullptr, bool do_fsync = false,
+          bool syncdataonly = false);
   int64_t _preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
                                  int iovcnt, int64_t offset,
                                  bool write, bool clamp_to_int,

src/test/client/nonblocking.cc

@@ -18,6 +18,7 @@
 #include <string>
 
 #include <fmt/format.h>
+#include <sys/statvfs.h>
 
 #include "test/client/TestClient.h"
 
@@ -716,3 +717,74 @@ TEST_F(TestClient, LlreadvContiguousLlwritevNonContiguous) {
   client->ll_release(fh);
   ASSERT_EQ(0, client->ll_unlink(root, filename, myperm));        
 }
+
+TEST_F(TestClient, LlreadvLlwritevLargeBuffers) {
+  /* Test that async I/O code paths handle large buffers (total len >= 4GiB)*/
+  int mypid = getpid();
+  char filename[256];
+
+  client->unmount();
+  TearDown();
+  SetUp();
+
+  sprintf(filename, "test_llreadvllwritevlargebuffers%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));
+
+  struct statvfs stbuf;
+  int64_t rc;
+  rc = client->ll_statfs(root, &stbuf, myperm);
+  ASSERT_EQ(rc, 0);
+  int64_t fs_available_space = stbuf.f_bfree * stbuf.f_bsize;
+  ASSERT_GT(fs_available_space, 0);
+
+  const size_t BUFSIZE = (size_t)INT_MAX + 1;
+  int64_t bytes_written = 0, bytes_read = 0;
+
+  C_SaferCond writefinish;
+  C_SaferCond readfinish;
+
+  auto out_buf_0 = std::make_unique<char[]>(BUFSIZE);
+  memset(out_buf_0.get(), 0xDD, BUFSIZE);
+  auto out_buf_1 = std::make_unique<char[]>(BUFSIZE);
+  memset(out_buf_1.get(), 0xFF, BUFSIZE);
+
+  struct iovec iov_out[2] = {
+    {out_buf_0.get(), BUFSIZE},
+    {out_buf_1.get(), BUFSIZE}
+  };
+
+  bufferlist bl;
+  auto in_buf_0 = std::make_unique<char[]>(BUFSIZE);
+  auto in_buf_1 = std::make_unique<char[]>(BUFSIZE);
+
+  struct iovec iov_in[2] = {
+    {in_buf_0.get(), BUFSIZE},
+    {in_buf_1.get(), BUFSIZE}
+  };
+
+  rc = client->ll_preadv_pwritev(fh, iov_out, 2, 0, true, &writefinish,
+                                 nullptr);
+  ASSERT_EQ(rc, 0);
+  bytes_written = writefinish.wait();
+  // total write length is clamped to INT_MAX in write paths
+  ASSERT_EQ(bytes_written, INT_MAX);
+
+  rc = client->ll_preadv_pwritev(fh, iov_in, 2, 0, false, &readfinish, &bl);
+  ASSERT_EQ(rc, 0);
+  bytes_read = readfinish.wait();
+  // total read length is clamped to INT_MAX in read paths
+  ASSERT_EQ(bytes_read, INT_MAX);
+
+  client->ll_release(fh);
+  ASSERT_EQ(0, client->ll_unlink(root, filename, myperm));
+}

src/test/client/syncio.cc

@@ -18,6 +18,7 @@
 #include <string>
 
 #include <fmt/format.h>
+#include <sys/statvfs.h>
 
 #include "test/client/TestClient.h"
 
@@ -77,3 +78,64 @@ TEST_F(TestClient, LlreadvLlwritevInvalidFileHandleSync) {
     rc = client->ll_readv(fh, iov_in, 2, 0);
     ASSERT_EQ(rc, -EBADF);
 }
+
+TEST_F(TestClient, LlreadvLlwritevLargeBuffersSync) {
+  /* Test that sync I/O code paths handle large buffers (total len >= 4GiB)*/
+  int mypid = getpid();
+  char filename[256];
+
+  client->unmount();
+  TearDown();
+  SetUp();
+
+  sprintf(filename, "test_llreadvllwritevlargebufferssync%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));
+
+  struct statvfs stbuf;
+  int64_t rc;
+  const size_t BUFSIZE = (size_t)INT_MAX + 1;
+  rc = client->ll_statfs(root, &stbuf, myperm);
+  ASSERT_EQ(rc, 0);
+  int64_t fs_available_space = stbuf.f_bfree * stbuf.f_bsize;
+  ASSERT_GT(fs_available_space, BUFSIZE * 2);
+
+  auto out_buf_0 = std::make_unique<char[]>(BUFSIZE);
+  memset(out_buf_0.get(), 0xDD, BUFSIZE);
+  auto out_buf_1 = std::make_unique<char[]>(BUFSIZE);
+  memset(out_buf_1.get(), 0xFF, BUFSIZE);
+
+  struct iovec iov_out[2] = {
+    {out_buf_0.get(), BUFSIZE},
+    {out_buf_1.get(), BUFSIZE}
+  };
+
+  bufferlist bl;
+  auto in_buf_0 = std::make_unique<char[]>(BUFSIZE);
+  auto in_buf_1 = std::make_unique<char[]>(BUFSIZE);
+
+  struct iovec iov_in[2] = {
+    {in_buf_0.get(), BUFSIZE},
+    {in_buf_1.get(), BUFSIZE}
+  };
+
+  rc = client->ll_writev(fh, iov_out, 2, 0);
+  // total write length is clamped to INT_MAX in write paths
+  ASSERT_EQ(rc, INT_MAX);
+
+  rc = client->ll_readv(fh, iov_in, 2, 0);
+  // total write length is clamped to INT_MAX in write paths
+  ASSERT_EQ(rc, INT_MAX);
+
+  client->ll_release(fh);
+  ASSERT_EQ(0, client->ll_unlink(root, filename, myperm));
+}