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

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 can be
  hit by continuously reading a file while, on another client, we keep
  truncating and writing new data into it.
- This resolves multiple issues caused by integer and consequent buffer
  overflow (`pages` array being accessed beyond `num_pages`):
  - https://tracker.ceph.com/issues/67524
  - https://tracker.ceph.com/issues/68980
  - https://tracker.ceph.com/issues/68981

Cc: stable@vger.kernel.org
Fixes: 1065da21e5 ("ceph: stop copying to iter at EOF on sync reads")
Reported-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
Signed-off-by: Alex Markuze <amarkuze@redhat.com>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This commit is contained in:
Alex Markuze 2024-11-27 15:34:10 +02:00 committed by Ilya Dryomov
parent 12eb22a5a6
commit 9abee47580

View File

@ -1066,7 +1066,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
if (ceph_inode_is_shutdown(inode)) if (ceph_inode_is_shutdown(inode))
return -EIO; return -EIO;
if (!len) if (!len || !i_size)
return 0; return 0;
/* /*
* flush any page cache pages in this range. this * 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; int num_pages;
size_t page_off; size_t page_off;
bool more; bool more;
int idx; int idx = 0;
size_t left; size_t left;
struct ceph_osd_req_op *op; struct ceph_osd_req_op *op;
u64 read_off = off; u64 read_off = off;
@ -1160,7 +1160,14 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
else if (ret == -ENOENT) else if (ret == -ENOENT)
ret = 0; 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; int fret;
fret = ceph_fscrypt_decrypt_extents(inode, pages, 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. */ /* 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 zlen = min(len - ret, i_size - off - ret);
int zoff = page_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; ret += zlen;
} }
idx = 0; if (off + ret > i_size)
if (ret <= 0) left = (i_size > off) ? i_size - off : 0;
left = 0;
else if (off + ret > i_size)
left = i_size - off;
else else
left = ret; left = ret;
while (left > 0) { while (left > 0) {
size_t plen, copied; 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); ceph_osdc_put_request(req);
if (ret < 0) {
if (ret == -EBLOCKLISTED)
fsc->blocklisted = true;
break;
}
if (off >= i_size || !more) if (off >= i_size || !more)
break; break;
} }