`iicrdwr` in `/sys/dev/iicbus/iic.c` incorrectly handles iteration over buffer. Firstly, no bound checks are supplied on the user controlled `d->nmsgs`. This field is declared as type `uint32_t`, in `struct iic_rdwr_data` (`sys/dev/iicbus/iic.h`): struct iic_rdwr_data { struct iic_msg *msgs; uint32_t nmsgs; }; However, the `i` variable in this function is declared as a `signed int`: int error, i; When `i` iterates over buffers, since it is `signed`, it can wrap around to a negative value, for example here: for (i = 0; i < d->nmsgs; i++) { m = &(buf[i]); usrbufs[i] = m->buf; And here: for (i = 0; i < d->nmsgs; i++) { m = &(buf[i]); if ((error == 0) && (m->flags & IIC_M_RD)) error = copyout(m->buf, usrbufs[i], m->len); free(m->buf, M_IIC); } `i` will be converted to `unsigned` type for the conversion, however, will still be `signed` when indexing `buf`. This would result in a read out of bounds of the `buf` allocation. This situation seems unlikely to be triggerable, because the code would wait for `buf` allocation to succeed (`M_WAITOK`): buf = malloc(sizeof(*d->msgs) * d->nmsgs, M_IIC, M_WAITOK); Which would be unlikely to succeed if `d->nmsgs` is something like `0x80000001`.
Potential patch is to change the type of `i` from `int` to `uint32_t` to match that of `d->nmsgs`: https://github.com/HardenedBSD/hardenedBSD-playground/commit/4cf6de4b16eda71c4ae3cbec24cf6ef054351b7b.patch However, some bound checks might be a cleaner solution.
Ive made patch based on CTurt's for this and issue#206755 waiting to be reviewed and approved.
A commit references this bug: Author: ngie Date: Sat Jan 30 18:33:24 UTC 2016 New revision: 295080 URL: https://svnweb.freebsd.org/changeset/base/295080 Log: Use the correct type for i when iterating over `buf` to avoid unlikely negative array indexing in iicrdwr(..) Differential Revision: https://reviews.freebsd.org/D5132 Obtained from: HardenedBSD PR: 206754 Reported by: CTurt <cturt@hardenedbsd.org> Submitted by: Madhi Moktari <mokhi64@gmail.com> Sponsored by: EMC / Isilon Storage Division Changes: head/sys/dev/iicbus/iic.c
Thanks for the submission! Will work on MFCing back to stable/10 and stable/9 as needed.
(In reply to Ngie Cooper from comment #4) @Ngie: will it be MFC'ed ?
(In reply to Mahdi Mokhtari from comment #5) This item hasn't been MFCed yet due to other QA issues with the module. Will open another bug with more info.
(In reply to Ngie Cooper from comment #6) Okay. Thanks for assigning it to yourself :) Then, please make this issue depend to that one you'll create.
Untaking bug -- I don't have the hardware to test out this component and this driver has other architectural issues that I don't want to get stuck dealing with.
The fix was committed as base r300258. Closing as Fixed