--- 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)