Skip to content

Commit 9abee47

Browse files
Alex Markuzeidryomov
authored andcommitted
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: [email protected] Fixes: 1065da2 ("ceph: stop copying to iter at EOF on sync reads") Reported-by: Luis Henriques (SUSE) <[email protected]> Signed-off-by: Alex Markuze <[email protected]> Reviewed-by: Viacheslav Dubeyko <[email protected]> Signed-off-by: Ilya Dryomov <[email protected]>
1 parent 12eb22a commit 9abee47

File tree

1 file changed

+14
-15
lines changed

1 file changed

+14
-15
lines changed

fs/ceph/file.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,7 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
10661066
if (ceph_inode_is_shutdown(inode))
10671067
return -EIO;
10681068

1069-
if (!len)
1069+
if (!len || !i_size)
10701070
return 0;
10711071
/*
10721072
* 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,
10861086
int num_pages;
10871087
size_t page_off;
10881088
bool more;
1089-
int idx;
1089+
int idx = 0;
10901090
size_t left;
10911091
struct ceph_osd_req_op *op;
10921092
u64 read_off = off;
@@ -1160,7 +1160,14 @@ ssize_t __ceph_sync_read(struct inode *inode, loff_t *ki_pos,
11601160
else if (ret == -ENOENT)
11611161
ret = 0;
11621162

1163-
if (ret > 0 && IS_ENCRYPTED(inode)) {
1163+
if (ret < 0) {
1164+
ceph_osdc_put_request(req);
1165+
if (ret == -EBLOCKLISTED)
1166+
fsc->blocklisted = true;
1167+
break;
1168+
}
1169+
1170+
if (IS_ENCRYPTED(inode)) {
11641171
int fret;
11651172

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

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

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

1200-
idx = 0;
1201-
if (ret <= 0)
1202-
left = 0;
1203-
else if (off + ret > i_size)
1204-
left = i_size - off;
1207+
if (off + ret > i_size)
1208+
left = (i_size > off) ? i_size - off : 0;
12051209
else
12061210
left = ret;
1211+
12071212
while (left > 0) {
12081213
size_t plen, copied;
12091214

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

12231228
ceph_osdc_put_request(req);
12241229

1225-
if (ret < 0) {
1226-
if (ret == -EBLOCKLISTED)
1227-
fsc->blocklisted = true;
1228-
break;
1229-
}
1230-
12311230
if (off >= i_size || !more)
12321231
break;
12331232
}

0 commit comments

Comments
 (0)