DF-1109 / fix.diff
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); |