⬢ DragonFlyBSD Kernel Audit
DF-5523 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/iicbus/iicsmb.c
+++ b/sys/bus/iicbus/iicsmb.c
@@ -465,6 +465,9 @@
 	device_t parent = device_get_parent(dev);
 	int error, sent;
 
+	if (count < 1 || count > 32)		/* SMBus spec 2.0 block limit */
+		return (SMB_EINVAL);
+
 	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		goto error;
 
@@ -486,6 +489,16 @@
 {
 	device_t parent = device_get_parent(dev);
 	int error, sent, read;
+
+	/*
+	 * Validate the caller-supplied count before it becomes a transfer
+	 * length: /dev/smb's smbioctl() passes an UNINITIALIZED stack byte
+	 * here (smb.c `u_char bcount;` is never assigned before
+	 * smbus_bread), so without this bound an uninitialized stack value
+	 * (observed: 0 and 255) drives the on-wire transfer length.
+	 */
+	if (*count < 1 || *count > 32)
+		return (SMB_EINVAL);
 
 	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		return (error);

--- a/sys/dev/smbus/smb/smb.c
+++ b/sys/dev/smbus/smb/smb.c
@@ -320,6 +320,13 @@
 			s->rcount = 0;
 		if (s->rcount > SMB_MAXBLOCKSIZE)
 			s->rcount = SMB_MAXBLOCKSIZE;
+		/*
+		 * bcount is an in/out parameter of smbus_bread(); it was never
+		 * initialized, so an uninitialized kernel stack byte selected the
+		 * transfer length and leaked back to userland via s->rcount.
+		 * Seed it from the requested count.
+		 */
+		bcount = s->rcount;
 		error = smbus_bread(bus, s->slave, s->cmd, &bcount, buf);
 		error = smbus_error(error);
 		if (error)