On Mon, 26 Mar 2018 14:27:52 -0700 Martin Kelly <mkelly@xxxxxxxx> wrote: > Currently, the following causes a kernel OOPS in memcpy: > > echo 1073741825 > buffer/length > echo 1 > buffer/enable > > Note that using 1073741824 instead of 1073741825 causes "write error: > Cannot allocate memory" but no OOPS. > > This is because 1073741824 == 2^30 and 1073741825 == 2^30+1. Since kfifo > rounds up to the nearest power of 2, it will actually call kmalloc with > roundup_pow_of_two(length) * bytes_per_datum. > > Using length == 1073741825 and bytes_per_datum == 2, we get: > > kmalloc(roundup_pow_of_two(1073741825) * 2 > or kmalloc(2147483648 * 2) > or kmalloc(4294967296) > or kmalloc(UINT_MAX + 1) > > so this overflows to 0, causing kmalloc to return ZERO_SIZE_PTR and > subsequent memcpy to fail once the device is enabled. > > Fix this by checking for overflow prior to allocating a kfifo. With this > check added, the above code returns -EINVAL when enabling the buffer, > rather than causing an OOPS. > > Signed-off-by: Martin Kelly <mkelly@xxxxxxxx> Applied to the fixes-togreg branch of iio.git and marked for stable. I thought about this for a few mins. A 4 gig allocation on a 32bit machine is going to fail anyway for obvious reasons, but good to protect against overflow if someone were to write this value. Particularly good description btw! Thanks, Jonathan > --- > drivers/iio/buffer/kfifo_buf.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c > index ac622edf2486..70c302a93d7f 100644 > --- a/drivers/iio/buffer/kfifo_buf.c > +++ b/drivers/iio/buffer/kfifo_buf.c > @@ -27,6 +27,13 @@ static inline int __iio_allocate_kfifo(struct iio_kfifo *buf, > if ((length == 0) || (bytes_per_datum == 0)) > return -EINVAL; > > + /* > + * Make sure we don't overflow an unsigned int after kfifo rounds up to > + * the next power of 2. > + */ > + if (roundup_pow_of_two(length) > UINT_MAX / bytes_per_datum) > + return -EINVAL; > + > return __kfifo_alloc((struct __kfifo *)&buf->kf, length, > bytes_per_datum, GFP_KERNEL); > } -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
