diff --git a/sys/dev/misc/ipmi/ipmi_ssif.c b/sys/dev/misc/ipmi/ipmi_ssif.c index e4f0558..f3c7382 100644 @@ -125,14 +125,22 @@ len = req->ir_requestlen - (SMBUS_DATA_SIZE - 2); cp = req->ir_request + (SMBUS_DATA_SIZE - 2); - while (len > 0) { + /* + * Send full SMBUS_DATA_SIZE-byte WRITE_CONT blocks until fewer + * than SMBUS_DATA_SIZE bytes remain. Note that len and cp MUST + * be advanced by the amount actually written; the previous code + * unconditionally subtracted SMBUS_DATA_SIZE, which underflowed + * the unsigned (size_t) len whenever the trailing partial block + * was shorter than SMBUS_DATA_SIZE, turning the loop into an + * unbounded kernel heap OOB read. + */ + while (len >= SMBUS_DATA_SIZE) { #ifdef SSIF_DEBUG - dump_buffer(dev, "WRITE_CONT", cp, - min(len, SMBUS_DATA_SIZE)); + dump_buffer(dev, "WRITE_CONT", cp, SMBUS_DATA_SIZE); #endif error = smbus_error(smbus_bwrite(smbus, sc->ipmi_ssif_smbus_address, SMBUS_WRITE_CONT, - min(len, SMBUS_DATA_SIZE), cp)); + SMBUS_DATA_SIZE, cp)); if (error) { #ifdef SSIF_ERROR_DEBUG device_printf(dev, "SSIF: WRITE_CONT error %d\n", @@ -145,13 +153,26 @@ } /* - * The final WRITE_CONT transaction has to have a non-zero - * length that is also not SMBUS_DATA_SIZE. If our last - * WRITE_CONT transaction in the loop sent SMBUS_DATA_SIZE - * bytes, then len will be 0, and we send an extra 0x00 byte - * to terminate the transaction. + * The terminating WRITE_CONT must be non-zero and shorter than + * SMBUS_DATA_SIZE. If any bytes remain (1..31) they terminate + * the transfer; otherwise send a single 0x00 byte. This is only + * reached for multi-part writes (requestlen > 30). */ - if (len == 0) { + if (len > 0) { +#ifdef SSIF_DEBUG + dump_buffer(dev, "WRITE_CONT", cp, len); +#endif + error = smbus_error(smbus_bwrite(smbus, + sc->ipmi_ssif_smbus_address, SMBUS_WRITE_CONT, + (u_char)len, cp)); + if (error) { +#ifdef SSIF_ERROR_DEBUG + device_printf(dev, "SSIF: WRITE_CONT error %d\n", + error); +#endif + goto fail; + } + } else { char c = 0; #ifdef SSIF_DEBUG