DragonFlyBSD Kernel Audit
DF-2461 / fix.diff
← back to finding ↓ download raw
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)) {