DragonFlyBSD Kernel Audit
DF-1109 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/bus/iicbus/iic.c b/sys/bus/iicbus/iic.c
--- a/sys/bus/iicbus/iic.c
+++ b/sys/bus/iicbus/iic.c
@@ -347,23 +347,37 @@
 		error = copyout(buf, s->buf, s->count);
 		break;
 
-	case I2CRDWR:
-		buf = kmalloc(sizeof(*d->msgs) * d->nmsgs, M_TEMP, M_WAITOK);
-		usrbufs = kmalloc(sizeof(void *) * d->nmsgs, M_TEMP, M_ZERO | M_WAITOK);
-		error = copyin(d->msgs, buf, sizeof(*d->msgs) * d->nmsgs);
+	case I2CRDWR: {
+		/*
+		 * Snapshot d->nmsgs ONCE into a local.  `d` is the raw user
+		 * ioctl-data pointer (drivers receive the un-copied struct), so
+		 * every `d->nmsgs` dereference re-fetches from user memory.  A
+		 * racing thread bumping nmsgs between the kmalloc and the
+		 * copyin/loops caused a heap overflow (16 bytes/excess entry in
+		 * buf, 8 in usrbufs).  Cap nmsgs to a sane upper bound to also
+		 * avoid multi-GB M_WAITOK allocations (OOM DoS).
+		 */
+		uint32_t nmsgs = d->nmsgs;
+		if (nmsgs == 0 || nmsgs > 65536) {
+			error = EINVAL;
+			break;
+		}
+		buf = kmalloc(sizeof(*d->msgs) * nmsgs, M_TEMP, M_WAITOK);
+		usrbufs = kmalloc(sizeof(void *) * nmsgs, M_TEMP, M_ZERO | M_WAITOK);
+		error = copyin(d->msgs, buf, sizeof(*d->msgs) * nmsgs);
 		if (error)
 			break;
 		/* Alloc kernel buffers for userland data, copyin write data */
-		for (i = 0; i < d->nmsgs; i++) {
+		for (i = 0; i < nmsgs; i++) {
 			m = &((struct iic_msg *)buf)[i];
 			usrbufs[i] = m->buf;
 			m->buf = kmalloc(m->len, M_TEMP, M_WAITOK);
 			if (!(m->flags & IIC_M_RD))
 				copyin(usrbufs[i], m->buf, m->len);
 		}
-		error = iicbus_transfer(parent, (struct iic_msg *)buf, d->nmsgs);
+		error = iicbus_transfer(parent, (struct iic_msg *)buf, nmsgs);
 		/* Copyout all read segments, free up kernel buffers */
-		for (i = 0; i < d->nmsgs; i++) {
+		for (i = 0; i < nmsgs; i++) {
 			m = &((struct iic_msg *)buf)[i];
 			if (m->flags & IIC_M_RD)
 				copyout(m->buf, usrbufs[i], m->len);
@@ -371,6 +385,7 @@
 		}
 		kfree(usrbufs, M_TEMP);
 		break;
+	}
 
 	case I2CRPTSTART:
 		error = iicbus_repeated_start(parent, s->slave, 0);