DF-2461 / fix.diff
diff --git a/sys/dev/disk/iscsi/initiator/isc_subr.c b/sys/dev/disk/iscsi/initiator/isc_subr.c --- a/sys/dev/disk/iscsi/initiator/isc_subr.c +++ b/sys/dev/disk/iscsi/initiator/isc_subr.c @@ -96,7 +96,14 @@ sp->opt.maxXmitDataSegmentLength = opt->maxXmitDataSegmentLength; sdebug(2, "maXmitDataSegmentLength=%d", sp->opt.maxXmitDataSegmentLength); } - if(opt->maxBurstLength != 0) { + if(opt->maxBurstLength > 0) { + /* + | maxBurstLength is a byte count; reject <= 0. A negative value + | used to be accepted (only '!= 0' was guarded) which then made + | i_prepPDU()'s 'len > sp->opt.maxBurstLength' comparison -- with + | len being size_t -- sign-extend the negative int to a huge + | unsigned value, silently disabling the E2BIG safety net. + */ sp->opt.maxBurstLength = opt->maxBurstLength; sdebug(2, "maxBurstLength=%d", sp->opt.maxBurstLength); } diff --git a/sys/dev/disk/iscsi/initiator/isc_sm.c b/sys/dev/disk/iscsi/initiator/isc_sm.c --- a/sys/dev/disk/iscsi/initiator/isc_sm.c +++ b/sys/dev/disk/iscsi/initiator/isc_sm.c @@ -288,6 +288,17 @@ len += 4; } + /* + | pq->len is a 32-bit u_int; 'len' is size_t (64-bit). Reject any + | PDU whose total length does not fit in 32 bits -- otherwise the + | assignment below silently truncates, i_send() kmalloc()s a tiny + | buffer while copyin() still uses the full ahs_len/ds_len, i.e. a + | kernel heap overflow. + */ + if (len > 0xffffffffUL) { + xdebug("%d] pdu len=%zd exceeds 32 bits", sp->sid, len); + return E2BIG; + } pq->len = len; len -= sizeof(bhs_t); if(sp->opt.maxBurstLength && (len > sp->opt.maxBurstLength)) { |