On Mon, Jun 08, 2020 at 03:06:40PM +0100, Jonathan Cameron wrote: > On Mon, 8 Jun 2020 16:14:58 +0300 > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > On Sun, Jun 07, 2020 at 04:54:03PM +0100, Jonathan Cameron 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. > > > > > + /* > > > + * Used to correctly align data. > > > + * Ensure timestamp is naturally aligned. > > > + */ > > > > > + u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u16)] __aligned(8); > > > > Can't you rather provide a struct as well? > > > Not without giving a false impression of where the time stamp is in the resulting > buffer. > > I'm not keen to do that because it'll lead to people fundamentally misunderstanding > the dynamic nature of IIO buffer packing. > > Here it could start at byte 8, 16, 24, 32, 48, 64 I think. I see, thanks for explanation! Same for the other comment. -- With Best Regards, Andy Shevchenko
