Skip to content

Commit a282a2f

Browse files
committed
libceph: harden msgr2.1 frame segment length checks
ceph_frame_desc::fd_lens is an int array. decode_preamble() thus effectively casts u32 -> int but the checks for segment lengths are written as if on unsigned values. While reading in HELLO or one of the AUTH frames (before authentication is completed), arithmetic in head_onwire_len() can get duped by negative ctrl_len and produce head_len which is less than CEPH_PREAMBLE_LEN but still positive. This would lead to a buffer overrun in prepare_read_control() as the preamble gets copied to the newly allocated buffer of size head_len. Cc: [email protected] Fixes: cd1a677 ("libceph, ceph: implement msgr2.1 protocol (crc and secure modes)") Reported-by: Thelford Williams <[email protected]> Signed-off-by: Ilya Dryomov <[email protected]> Reviewed-by: Xiubo Li <[email protected]>
1 parent 06c2afb commit a282a2f

File tree

1 file changed

+26
-15
lines changed

1 file changed

+26
-15
lines changed

net/ceph/messenger_v2.c

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,8 @@ static int head_onwire_len(int ctrl_len, bool secure)
390390
int head_len;
391391
int rem_len;
392392

393+
BUG_ON(ctrl_len < 0 || ctrl_len > CEPH_MSG_MAX_CONTROL_LEN);
394+
393395
if (secure) {
394396
head_len = CEPH_PREAMBLE_SECURE_LEN;
395397
if (ctrl_len > CEPH_PREAMBLE_INLINE_LEN) {
@@ -408,6 +410,10 @@ static int head_onwire_len(int ctrl_len, bool secure)
408410
static int __tail_onwire_len(int front_len, int middle_len, int data_len,
409411
bool secure)
410412
{
413+
BUG_ON(front_len < 0 || front_len > CEPH_MSG_MAX_FRONT_LEN ||
414+
middle_len < 0 || middle_len > CEPH_MSG_MAX_MIDDLE_LEN ||
415+
data_len < 0 || data_len > CEPH_MSG_MAX_DATA_LEN);
416+
411417
if (!front_len && !middle_len && !data_len)
412418
return 0;
413419

@@ -520,29 +526,34 @@ static int decode_preamble(void *p, struct ceph_frame_desc *desc)
520526
desc->fd_aligns[i] = ceph_decode_16(&p);
521527
}
522528

523-
/*
524-
* This would fire for FRAME_TAG_WAIT (it has one empty
525-
* segment), but we should never get it as client.
526-
*/
527-
if (!desc->fd_lens[desc->fd_seg_cnt - 1]) {
528-
pr_err("last segment empty\n");
529+
if (desc->fd_lens[0] < 0 ||
530+
desc->fd_lens[0] > CEPH_MSG_MAX_CONTROL_LEN) {
531+
pr_err("bad control segment length %d\n", desc->fd_lens[0]);
529532
return -EINVAL;
530533
}
531-
532-
if (desc->fd_lens[0] > CEPH_MSG_MAX_CONTROL_LEN) {
533-
pr_err("control segment too big %d\n", desc->fd_lens[0]);
534+
if (desc->fd_lens[1] < 0 ||
535+
desc->fd_lens[1] > CEPH_MSG_MAX_FRONT_LEN) {
536+
pr_err("bad front segment length %d\n", desc->fd_lens[1]);
534537
return -EINVAL;
535538
}
536-
if (desc->fd_lens[1] > CEPH_MSG_MAX_FRONT_LEN) {
537-
pr_err("front segment too big %d\n", desc->fd_lens[1]);
539+
if (desc->fd_lens[2] < 0 ||
540+
desc->fd_lens[2] > CEPH_MSG_MAX_MIDDLE_LEN) {
541+
pr_err("bad middle segment length %d\n", desc->fd_lens[2]);
538542
return -EINVAL;
539543
}
540-
if (desc->fd_lens[2] > CEPH_MSG_MAX_MIDDLE_LEN) {
541-
pr_err("middle segment too big %d\n", desc->fd_lens[2]);
544+
if (desc->fd_lens[3] < 0 ||
545+
desc->fd_lens[3] > CEPH_MSG_MAX_DATA_LEN) {
546+
pr_err("bad data segment length %d\n", desc->fd_lens[3]);
542547
return -EINVAL;
543548
}
544-
if (desc->fd_lens[3] > CEPH_MSG_MAX_DATA_LEN) {
545-
pr_err("data segment too big %d\n", desc->fd_lens[3]);
549+
550+
/*
551+
* This would fire for FRAME_TAG_WAIT (it has one empty
552+
* segment), but we should never get it as client.
553+
*/
554+
if (!desc->fd_lens[desc->fd_seg_cnt - 1]) {
555+
pr_err("last segment empty, segment count %d\n",
556+
desc->fd_seg_cnt);
546557
return -EINVAL;
547558
}
548559

0 commit comments

Comments
 (0)