On Wed, 22 Jul 2020 23:54:29 +0300 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Wed, Jul 22, 2020 at 6:53 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > > > One of a class of bugs pointed out by Lars in a recent review. > > iio_push_to_buffers_with_timestamp assumes the buffer used is aligned > > to the size of the timestamp (8 bytes). This is not guaranteed in > > this driver which uses an array of smaller elements on the stack. > > As Lars also noted this anti pattern can involve a leak of data to > > userspace and that indeed can happen here. We close both issues by > > moving to a suitable structure in the iio_priv() data with alignment > > explicitly requested. This data is allocated with kzalloc so no > > data can leak apart from previous readings. > > > > In this driver the timestamp can end up in various different locations > > depending on what other channels are enabled. As a result, we don't > > use a structure to specify it's position as that would be missleading. > > ... > > > + /* > > + * Used to correctly align data. > > + * Ensure timestamp is naturally aligned. > > + */ > > + u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u16)] __aligned(8); > > u32 vs. u16? > Curious indeed. My eyes jumped straight over that when cutting and pasting that line. I guess too big is never a problem, but should definitely tidy that up. Thanks, Jonathan
