Re: [bug report] iio: pressure: bmp280: drop sensor_data array

[Dan Carpenter <dan.carpenter@linaro.org>]

On Wed, May 07, 2025 at 07:35:27AM +0100, Jonathan Cameron wrote:
> 
> 
> On 6 May 2025 19:35:08 BST, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >On Tue, May 06, 2025 at 09:25:00AM -0500, David Lechner wrote:
> >> On 5/6/25 7:32 AM, Dan Carpenter wrote:
> >> > Hello David Lechner,
> >> > 
> >> > Commit 4e6c3c4801a6 ("iio: pressure: bmp280: drop sensor_data array")
> >> > from Apr 22, 2025 (linux-next), leads to the following Smatch static
> >> > checker warning:
> >> > 
> >> > 	drivers/iio/pressure/bmp280-core.c:1280 bme280_trigger_handler()
> >> > 	warn: check that 'buffer' doesn't leak information (struct has a hole after 'comp_humidity')
> >> > 
> >> > drivers/iio/pressure/bmp280-core.c
> >> >     1225 static irqreturn_t bme280_trigger_handler(int irq, void *p)
> >> >     1226 {
> >> >     1227         struct iio_poll_func *pf = p;
> >> >     1228         struct iio_dev *indio_dev = pf->indio_dev;
> >> >     1229         struct bmp280_data *data = iio_priv(indio_dev);
> >> >     1230         u32 adc_temp, adc_press, adc_humidity;
> >> >     1231         s32 t_fine;
> >> >     1232         struct {
> >> >     1233                 u32 comp_press;
> >> >     1234                 s32 comp_temp;
> >> >     1235                 u32 comp_humidity;
> >> >     1236                 aligned_s64 timestamp;
> >> > 
> >> > There is a 4 byte hole between comp_humidity and timestamp.
> >> 
> >> Yes, this was the intention of this patch.
> >> 
> >> > 
> >> >     1237         } buffer;
> >> >     1238         int ret;
> >> >     1239 
> >> 
> >> ...
> >> 
> >> >     1279 
> >> > --> 1280         iio_push_to_buffers_with_ts(indio_dev, &buffer, sizeof(buffer),
> >> >                                                         ^^^^^^^^^^^^^^^^^^^^^^^
> >> > So I believe it leads to an information leaks here.
> >> 
> >> Aha, so we should e.g. do memset() to fill the hole first.
> >> 
> >
> >That works, or you could initialize it with = { }.
> 
> I tried to track this down the other day.
> What are compilers guaranteeing around
> that vs { 0 }  and holes?  The c spec has only recently standardised on { }.
> 
> I'd love to stop using memset for these.

The rule in the C standard is that if the initializer sets every struct
member then it will NOT zero out struct holes.  But if there are any
unset struct members then the holes are zeroed.  So = { } will always
work.  You really have to try hard to invent a scenario where = { 0 }
won't fill the struct holes (a one member struct with a weird alignment).

regards,
dan carpenter