diff mbox series

ceph: improve error handling and short/overflow-read logic in __ceph_sync_read()

Message ID CAO8a2ShLJP686V7T6sRnS=GHtHAJASrk+_ZE6qFEja0mCwiXHQ@mail.gmail.com (mailing list archive)
State New
Headers show
Series ceph: improve error handling and short/overflow-read logic in __ceph_sync_read() | expand

Commit Message

Alex Markuze Dec. 9, 2024, 8:16 a.m. UTC
This patch refines the read logic in __ceph_sync_read() to ensure more
predictable and efficient behavior in various edge cases.

- Return early if the requested read length is zero or if the file size
  (`i_size`) is zero.
- Initialize the index variable (`idx`) where needed and reorder some
  code to ensure it is always set before use.
- Improve error handling by checking for negative return values earlier.
- Remove redundant encrypted file checks after failures. Only attempt
  filesystem-level decryption if the read succeeded.
- Simplify leftover calculations to correctly handle cases where the read
  extends beyond the end of the file or stops short.
- This resolves multiple issues caused by integer overflow
  - https://tracker.ceph.com/issues/67524
  - https://tracker.ceph.com/issues/68981
  - https://tracker.ceph.com/issues/68980

Signed-off-by: Alex Markuze <amarkuze@redhat.com>
---
 fs/ceph/file.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

          * flush any page cache pages in this range.  this
@@ -1086,7 +1086,7 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
                 int num_pages;
                 size_t page_off;
                 bool more;
-               int idx;
+               int idx = 0;
                 size_t left;
                 struct ceph_osd_req_op *op;
                 u64 read_off = off;
@@ -1160,7 +1160,14 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
                 else if (ret == -ENOENT)
                         ret = 0;

-               if (ret > 0 && IS_ENCRYPTED(inode)) {
+               if (ret < 0) {
+                       ceph_osdc_put_request(req);
+                       if (ret == -EBLOCKLISTED)
+                               fsc->blocklisted = true;
+                       break;
+               }
+
+               if (IS_ENCRYPTED(inode)) {
                         int fret;

                         fret = ceph_fscrypt_decrypt_extents(inode, pages,
@@ -1187,7 +1194,7 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
                 }

                 /* Short read but not EOF? Zero out the remainder. */
-               if (ret >= 0 && ret < len && (off + ret < i_size)) {
+               if (ret < len && (off + ret < i_size)) {
                         int zlen = min(len - ret, i_size - off - ret);
                         int zoff = page_off + ret;

@@ -1197,13 +1204,11 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
                         ret += zlen;
                 }

-               idx = 0;
-               if (ret <= 0)
-                       left = 0;
-               else if (off + ret > i_size)
-                       left = i_size - off;
+               if (off + ret > i_size)
+                       left = (i_size > off) ? i_size - off : 0;
                 else
                         left = ret;
+
                 while (left > 0) {
                         size_t plen, copied;

@@ -1222,12 +1227,6 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,

                 ceph_osdc_put_request(req);

-               if (ret < 0) {
-                       if (ret == -EBLOCKLISTED)
-                               fsc->blocklisted = true;
-                       break;
-               }
-
                 if (off >= i_size || !more)
                         break;
         }
--
2.34.1
diff mbox series

Patch

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index ce342a5d4b8b..8e0400d461a2 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1066,7 +1066,7 @@  ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
         if (ceph_inode_is_shutdown(inode))
                 return -EIO;

-       if (!len)
+       if (!len || !i_size)
                 return 0;
         /*