[PATCH 00/28] iio: zero init stack with { } instead of memset()

David Lechner posted 28 patches 4 months ago
drivers/iio/accel/adxl372.c                       | 3 +--
drivers/iio/accel/msa311.c                        | 4 +---
drivers/iio/adc/dln2-adc.c                        | 4 +---
drivers/iio/adc/mt6360-adc.c                      | 3 +--
drivers/iio/adc/rockchip_saradc.c                 | 4 +---
drivers/iio/adc/rtq6056.c                         | 4 +---
drivers/iio/adc/stm32-adc.c                       | 3 +--
drivers/iio/adc/ti-ads1015.c                      | 4 +---
drivers/iio/adc/ti-ads1119.c                      | 4 +---
drivers/iio/adc/ti-lmp92064.c                     | 4 +---
drivers/iio/adc/ti-tsc2046.c                      | 3 +--
drivers/iio/chemical/scd30_core.c                 | 3 +--
drivers/iio/chemical/scd4x.c                      | 3 +--
drivers/iio/chemical/sunrise_co2.c                | 6 ++----
drivers/iio/dac/ad3552r.c                         | 3 +--
drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
drivers/iio/light/bh1745.c                        | 4 +---
drivers/iio/light/ltr501.c                        | 4 +---
drivers/iio/light/opt4060.c                       | 4 +---
drivers/iio/light/veml6030.c                      | 4 +---
drivers/iio/magnetometer/af8133j.c                | 4 +---
drivers/iio/pressure/bmp280-core.c                | 5 +----
drivers/iio/pressure/mpl3115.c                    | 3 +--
drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
drivers/iio/pressure/zpa2326.c                    | 4 +---
drivers/iio/proximity/irsd200.c                   | 3 +--
drivers/iio/temperature/tmp006.c                  | 4 +---
30 files changed, 34 insertions(+), 85 deletions(-)
[PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by David Lechner 4 months ago
Jonathan mentioned recently that he would like to get away from using
memset() to zero-initialize stack memory in the IIO subsystem. And we
have it on good authority that initializing a struct or array with = { }
is the preferred way to do this in the kernel [1]. So here is a series
to take care of that.

[1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/

---
David Lechner (28):
      iio: accel: adxl372: use = { } instead of memset()
      iio: accel: msa311: use = { } instead of memset()
      iio: adc: dln2-adc: use = { } instead of memset()
      iio: adc: mt6360-adc: use = { } instead of memset()
      iio: adc: rockchip_saradc: use = { } instead of memset()
      iio: adc: rtq6056: use = { } instead of memset()
      iio: adc: stm32-adc: use = { } instead of memset()
      iio: adc: ti-ads1015: use = { } instead of memset()
      iio: adc: ti-ads1119: use = { } instead of memset()
      iio: adc: ti-lmp92064: use = { } instead of memset()
      iio: adc: ti-tsc2046: use = { } instead of memset()
      iio: chemical: scd4x: use = { } instead of memset()
      iio: chemical: scd30: use = { } instead of memset()
      iio: chemical: sunrise_co2: use = { } instead of memset()
      iio: dac: ad3552r: use = { } instead of memset()
      iio: imu: inv_icm42600: use = { } instead of memset()
      iio: imu: inv_mpu6050: use = { } instead of memset()
      iio: light: bh1745: use = { } instead of memset()
      iio: light: ltr501: use = { } instead of memset()
      iio: light: opt4060: use = { } instead of memset()
      iio: light: veml6030: use = { } instead of memset()
      iio: magnetometer: af8133j: use = { } instead of memset()
      iio: pressure: bmp280: use = { } instead of memset()
      iio: pressure: mpl3115: use = { } instead of memset()
      iio: pressure: mprls0025pa: use = { } instead of memset()
      iio: pressure: zpa2326: use = { } instead of memset()
      iio: proximity: irsd200: use = { } instead of memset()
      iio: temperature: tmp006: use = { } instead of memset()

 drivers/iio/accel/adxl372.c                       | 3 +--
 drivers/iio/accel/msa311.c                        | 4 +---
 drivers/iio/adc/dln2-adc.c                        | 4 +---
 drivers/iio/adc/mt6360-adc.c                      | 3 +--
 drivers/iio/adc/rockchip_saradc.c                 | 4 +---
 drivers/iio/adc/rtq6056.c                         | 4 +---
 drivers/iio/adc/stm32-adc.c                       | 3 +--
 drivers/iio/adc/ti-ads1015.c                      | 4 +---
 drivers/iio/adc/ti-ads1119.c                      | 4 +---
 drivers/iio/adc/ti-lmp92064.c                     | 4 +---
 drivers/iio/adc/ti-tsc2046.c                      | 3 +--
 drivers/iio/chemical/scd30_core.c                 | 3 +--
 drivers/iio/chemical/scd4x.c                      | 3 +--
 drivers/iio/chemical/sunrise_co2.c                | 6 ++----
 drivers/iio/dac/ad3552r.c                         | 3 +--
 drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
 drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
 drivers/iio/light/bh1745.c                        | 4 +---
 drivers/iio/light/ltr501.c                        | 4 +---
 drivers/iio/light/opt4060.c                       | 4 +---
 drivers/iio/light/veml6030.c                      | 4 +---
 drivers/iio/magnetometer/af8133j.c                | 4 +---
 drivers/iio/pressure/bmp280-core.c                | 5 +----
 drivers/iio/pressure/mpl3115.c                    | 3 +--
 drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
 drivers/iio/pressure/zpa2326.c                    | 4 +---
 drivers/iio/proximity/irsd200.c                   | 3 +--
 drivers/iio/temperature/tmp006.c                  | 4 +---
 30 files changed, 34 insertions(+), 85 deletions(-)
---
base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb

Best regards,
-- 
David Lechner <dlechner@baylibre.com>
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Pavel Machek 4 months ago
Hi!

> Jonathan mentioned recently that he would like to get away from using
> memset() to zero-initialize stack memory in the IIO subsystem. And we
> have it on good authority that initializing a struct or array with = { }
> is the preferred way to do this in the kernel [1]. So here is a series
> to take care of that.

1) Is it worth the churn?

2) Will this fail to initialize padding with some obscure compiler?

3) Why do you believe that {} is the preffered way? All we have is
Kees' email that explains that = {} maybe works in configs he tested.

BR,
								Pavel

> [1]:
> https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/



> ---
> David Lechner (28):
>       iio: accel: adxl372: use = { } instead of memset()
>       iio: accel: msa311: use = { } instead of memset()
>       iio: adc: dln2-adc: use = { } instead of memset()
>       iio: adc: mt6360-adc: use = { } instead of memset()
>       iio: adc: rockchip_saradc: use = { } instead of memset()
>       iio: adc: rtq6056: use = { } instead of memset()
>       iio: adc: stm32-adc: use = { } instead of memset()
>       iio: adc: ti-ads1015: use = { } instead of memset()
>       iio: adc: ti-ads1119: use = { } instead of memset()
>       iio: adc: ti-lmp92064: use = { } instead of memset()
>       iio: adc: ti-tsc2046: use = { } instead of memset()
>       iio: chemical: scd4x: use = { } instead of memset()
>       iio: chemical: scd30: use = { } instead of memset()
>       iio: chemical: sunrise_co2: use = { } instead of memset()
>       iio: dac: ad3552r: use = { } instead of memset()
>       iio: imu: inv_icm42600: use = { } instead of memset()
>       iio: imu: inv_mpu6050: use = { } instead of memset()
>       iio: light: bh1745: use = { } instead of memset()
>       iio: light: ltr501: use = { } instead of memset()
>       iio: light: opt4060: use = { } instead of memset()
>       iio: light: veml6030: use = { } instead of memset()
>       iio: magnetometer: af8133j: use = { } instead of memset()
>       iio: pressure: bmp280: use = { } instead of memset()
>       iio: pressure: mpl3115: use = { } instead of memset()
>       iio: pressure: mprls0025pa: use = { } instead of memset()
>       iio: pressure: zpa2326: use = { } instead of memset()
>       iio: proximity: irsd200: use = { } instead of memset()
>       iio: temperature: tmp006: use = { } instead of memset()
> 
>  drivers/iio/accel/adxl372.c                       | 3 +--
>  drivers/iio/accel/msa311.c                        | 4 +---
>  drivers/iio/adc/dln2-adc.c                        | 4 +---
>  drivers/iio/adc/mt6360-adc.c                      | 3 +--
>  drivers/iio/adc/rockchip_saradc.c                 | 4 +---
>  drivers/iio/adc/rtq6056.c                         | 4 +---
>  drivers/iio/adc/stm32-adc.c                       | 3 +--
>  drivers/iio/adc/ti-ads1015.c                      | 4 +---
>  drivers/iio/adc/ti-ads1119.c                      | 4 +---
>  drivers/iio/adc/ti-lmp92064.c                     | 4 +---
>  drivers/iio/adc/ti-tsc2046.c                      | 3 +--
>  drivers/iio/chemical/scd30_core.c                 | 3 +--
>  drivers/iio/chemical/scd4x.c                      | 3 +--
>  drivers/iio/chemical/sunrise_co2.c                | 6 ++----
>  drivers/iio/dac/ad3552r.c                         | 3 +--
>  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
>  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
>  drivers/iio/light/bh1745.c                        | 4 +---
>  drivers/iio/light/ltr501.c                        | 4 +---
>  drivers/iio/light/opt4060.c                       | 4 +---
>  drivers/iio/light/veml6030.c                      | 4 +---
>  drivers/iio/magnetometer/af8133j.c                | 4 +---
>  drivers/iio/pressure/bmp280-core.c                | 5 +----
>  drivers/iio/pressure/mpl3115.c                    | 3 +--
>  drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
>  drivers/iio/pressure/zpa2326.c                    | 4 +---
>  drivers/iio/proximity/irsd200.c                   | 3 +--
>  drivers/iio/temperature/tmp006.c                  | 4 +---
>  30 files changed, 34 insertions(+), 85 deletions(-)
> ---
> base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
> change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb
> 
> Best regards,

-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Nicolas Frattaroli 4 months ago
Hello,

I thought I'd chime in as someone uninvolved because this seemed
interesting.

On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:
> Hi!
> 
> > Jonathan mentioned recently that he would like to get away from using
> > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > have it on good authority that initializing a struct or array with = { }
> > is the preferred way to do this in the kernel [1]. So here is a series
> > to take care of that.
> 
> 1) Is it worth the churn?
> 
> 2) Will this fail to initialize padding with some obscure compiler?

as of right now, the only two C compilers that are supported are
GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the kernel
to finish a build with something else, I think the compiler not
implementing the C standard correctly is the least of their worries.

My bigger worry is that = { } is only guaranteed to be as correct as
memset on C23, and the kernel's standard right now is C11. For that
reason alone, I don't think memset should be moved away from for now,
unless someone can verify that every GCC release >= 8.1 and every
Clang release >= 13.0.1 does the right thing here regardless.

> 
> 3) Why do you believe that {} is the preffered way? All we have is
> Kees' email that explains that = {} maybe works in configs he tested.

= { } is guaranteed to work in C23, as per the standard, but again we're
not on C23.

The reason to prefer this is likely that it's easier for static analysis
to see the struct as initialised, but that's me making assumptions here.

A more human-centric argument is that once we're on a C standards version
where = { } is guaranteed to be correct, then = { } is much more obviously
correct to a reader than a memset with a value and a size somewhere later
in the code. This argument is evident from the number of patches in this
series where the memset and the declaration are not in the same hunk.
That's the kind of stuff that keeps me awake at night, sweating profusely.

Kind regards,
Nicolas Frattaroli

> 
> BR,
> 								Pavel
> 
> > [1]:
> > https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/
> 
> 
> 
> > ---
> > David Lechner (28):
> >       iio: accel: adxl372: use = { } instead of memset()
> >       iio: accel: msa311: use = { } instead of memset()
> >       iio: adc: dln2-adc: use = { } instead of memset()
> >       iio: adc: mt6360-adc: use = { } instead of memset()
> >       iio: adc: rockchip_saradc: use = { } instead of memset()
> >       iio: adc: rtq6056: use = { } instead of memset()
> >       iio: adc: stm32-adc: use = { } instead of memset()
> >       iio: adc: ti-ads1015: use = { } instead of memset()
> >       iio: adc: ti-ads1119: use = { } instead of memset()
> >       iio: adc: ti-lmp92064: use = { } instead of memset()
> >       iio: adc: ti-tsc2046: use = { } instead of memset()
> >       iio: chemical: scd4x: use = { } instead of memset()
> >       iio: chemical: scd30: use = { } instead of memset()
> >       iio: chemical: sunrise_co2: use = { } instead of memset()
> >       iio: dac: ad3552r: use = { } instead of memset()
> >       iio: imu: inv_icm42600: use = { } instead of memset()
> >       iio: imu: inv_mpu6050: use = { } instead of memset()
> >       iio: light: bh1745: use = { } instead of memset()
> >       iio: light: ltr501: use = { } instead of memset()
> >       iio: light: opt4060: use = { } instead of memset()
> >       iio: light: veml6030: use = { } instead of memset()
> >       iio: magnetometer: af8133j: use = { } instead of memset()
> >       iio: pressure: bmp280: use = { } instead of memset()
> >       iio: pressure: mpl3115: use = { } instead of memset()
> >       iio: pressure: mprls0025pa: use = { } instead of memset()
> >       iio: pressure: zpa2326: use = { } instead of memset()
> >       iio: proximity: irsd200: use = { } instead of memset()
> >       iio: temperature: tmp006: use = { } instead of memset()
> > 
> >  drivers/iio/accel/adxl372.c                       | 3 +--
> >  drivers/iio/accel/msa311.c                        | 4 +---
> >  drivers/iio/adc/dln2-adc.c                        | 4 +---
> >  drivers/iio/adc/mt6360-adc.c                      | 3 +--
> >  drivers/iio/adc/rockchip_saradc.c                 | 4 +---
> >  drivers/iio/adc/rtq6056.c                         | 4 +---
> >  drivers/iio/adc/stm32-adc.c                       | 3 +--
> >  drivers/iio/adc/ti-ads1015.c                      | 4 +---
> >  drivers/iio/adc/ti-ads1119.c                      | 4 +---
> >  drivers/iio/adc/ti-lmp92064.c                     | 4 +---
> >  drivers/iio/adc/ti-tsc2046.c                      | 3 +--
> >  drivers/iio/chemical/scd30_core.c                 | 3 +--
> >  drivers/iio/chemical/scd4x.c                      | 3 +--
> >  drivers/iio/chemical/sunrise_co2.c                | 6 ++----
> >  drivers/iio/dac/ad3552r.c                         | 3 +--
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
> >  drivers/iio/light/bh1745.c                        | 4 +---
> >  drivers/iio/light/ltr501.c                        | 4 +---
> >  drivers/iio/light/opt4060.c                       | 4 +---
> >  drivers/iio/light/veml6030.c                      | 4 +---
> >  drivers/iio/magnetometer/af8133j.c                | 4 +---
> >  drivers/iio/pressure/bmp280-core.c                | 5 +----
> >  drivers/iio/pressure/mpl3115.c                    | 3 +--
> >  drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
> >  drivers/iio/pressure/zpa2326.c                    | 4 +---
> >  drivers/iio/proximity/irsd200.c                   | 3 +--
> >  drivers/iio/temperature/tmp006.c                  | 4 +---
> >  30 files changed, 34 insertions(+), 85 deletions(-)
> > ---
> > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
> > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb
> > 
> > Best regards,
> 
>
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Pavel Machek 4 months ago
Hi1

> I thought I'd chime in as someone uninvolved because this seemed
> interesting.
> 
> On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:
> > Hi!
> > 
> > > Jonathan mentioned recently that he would like to get away from using
> > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > have it on good authority that initializing a struct or array with = { }
> > > is the preferred way to do this in the kernel [1]. So here is a series
> > > to take care of that.
> > 
> > 1) Is it worth the churn?
> > 
> > 2) Will this fail to initialize padding with some obscure compiler?
> 
> as of right now, the only two C compilers that are supported are
> GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the
> kernel

Well... I'm pretty sure parts of this would make it into -stable as a
dependency, or because AUTOSEL decides it is a bugfix. So..

GNU C                  4.9              gcc --version
Clang/LLVM (optional)  10.0.1           clang --version

:-).

Best regards,
									Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Andy Shevchenko 4 months ago
On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote:
> > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:
> > > 
> > > > Jonathan mentioned recently that he would like to get away from using
> > > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > > have it on good authority that initializing a struct or array with = { }
> > > > is the preferred way to do this in the kernel [1]. So here is a series
> > > > to take care of that.
> > > 
> > > 1) Is it worth the churn?
> > > 
> > > 2) Will this fail to initialize padding with some obscure compiler?
> > 
> > as of right now, the only two C compilers that are supported are
> > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the
> > kernel
> 
> Well... I'm pretty sure parts of this would make it into -stable as a
> dependency, or because AUTOSEL decides it is a bugfix. So..
> 
> GNU C                  4.9              gcc --version
> Clang/LLVM (optional)  10.0.1           clang --version

Even though, what the kernel versions are you referring to? I am sure there
plenty of cases with {} there.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Pavel Machek 3 months, 4 weeks ago
On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote:
> On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote:
> > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:
> > > > 
> > > > > Jonathan mentioned recently that he would like to get away from using
> > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > > > have it on good authority that initializing a struct or array with = { }
> > > > > is the preferred way to do this in the kernel [1]. So here is a series
> > > > > to take care of that.
> > > > 
> > > > 1) Is it worth the churn?
> > > > 
> > > > 2) Will this fail to initialize padding with some obscure compiler?
> > > 
> > > as of right now, the only two C compilers that are supported are
> > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the
> > > kernel
> > 
> > Well... I'm pretty sure parts of this would make it into -stable as a
> > dependency, or because AUTOSEL decides it is a bugfix. So..
> > 
> > GNU C                  4.9              gcc --version
> > Clang/LLVM (optional)  10.0.1           clang --version
> 
> Even though, what the kernel versions are you referring to? I am sure there
> plenty of cases with {} there.

5.10, for example. I'm sure they are, uninitialized padding is a
security hole, but rather hard to detect if they are not specifically
looking.

BR,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Jonathan Cameron 3 months, 4 weeks ago
On Sat, 14 Jun 2025 08:47:25 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote:
> > On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote:  
> > > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:  
> > > > >   
> > > > > > Jonathan mentioned recently that he would like to get away from using
> > > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > > > > have it on good authority that initializing a struct or array with = { }
> > > > > > is the preferred way to do this in the kernel [1]. So here is a series
> > > > > > to take care of that.  
> > > > > 
> > > > > 1) Is it worth the churn?
> > > > > 
> > > > > 2) Will this fail to initialize padding with some obscure compiler?  
> > > > 
> > > > as of right now, the only two C compilers that are supported are
> > > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the
> > > > kernel  
> > > 
> > > Well... I'm pretty sure parts of this would make it into -stable as a
> > > dependency, or because AUTOSEL decides it is a bugfix. So..
> > > 
> > > GNU C                  4.9              gcc --version
> > > Clang/LLVM (optional)  10.0.1           clang --version  
> > 
> > Even though, what the kernel versions are you referring to? I am sure there
> > plenty of cases with {} there.  
> 
> 5.10, for example. I'm sure they are, uninitialized padding is a
> security hole, but rather hard to detect if they are not specifically
> looking.

The stack kunit test is there back to 5.0-rc4 
50ceaa95ea09 ("lib: Introduce test_stackinit module")

So I think we should be pretty well defended against issues.

Hence I plan to pick this up curently.

Thanks all for inputs on this.

Fun corners of the C spec vs implementations!

Jonathan

> 
> BR,
> 								Pavel
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Jonathan Cameron 3 months, 4 weeks ago
On Sat, 14 Jun 2025 13:18:44 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 14 Jun 2025 08:47:25 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Thu 2025-06-12 22:10:07, Andy Shevchenko wrote:  
> > > On Thu, Jun 12, 2025 at 08:54:07PM +0200, Pavel Machek wrote:    
> > > > > On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:    
> > > > > >     
> > > > > > > Jonathan mentioned recently that he would like to get away from using
> > > > > > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > > > > > have it on good authority that initializing a struct or array with = { }
> > > > > > > is the preferred way to do this in the kernel [1]. So here is a series
> > > > > > > to take care of that.    
> > > > > > 
> > > > > > 1) Is it worth the churn?
> > > > > > 
> > > > > > 2) Will this fail to initialize padding with some obscure compiler?    
> > > > > 
> > > > > as of right now, the only two C compilers that are supported are
> > > > > GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the
> > > > > kernel    
> > > > 
> > > > Well... I'm pretty sure parts of this would make it into -stable as a
> > > > dependency, or because AUTOSEL decides it is a bugfix. So..
> > > > 
> > > > GNU C                  4.9              gcc --version
> > > > Clang/LLVM (optional)  10.0.1           clang --version    
> > > 
> > > Even though, what the kernel versions are you referring to? I am sure there
> > > plenty of cases with {} there.    
> > 
> > 5.10, for example. I'm sure they are, uninitialized padding is a
> > security hole, but rather hard to detect if they are not specifically
> > looking.  
> 
> The stack kunit test is there back to 5.0-rc4 
> 50ceaa95ea09 ("lib: Introduce test_stackinit module")
> 
> So I think we should be pretty well defended against issues.
> 
> Hence I plan to pick this up curently.
> 
> Thanks all for inputs on this.
> 
> Fun corners of the C spec vs implementations!
> 
> Jonathan
> 
I want to give this some testing exposure from 0-day etc in case
we missed any build related issues so I've queued it up on my testing branch.
I can still pick up tags / rebase etc for now.

Thanks,

Jonathan

> > 
> > BR,
> > 								Pavel  
> 
>
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Andy Shevchenko 4 months ago
On Thu, Jun 12, 2025 at 3:12 PM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:

> I thought I'd chime in as someone uninvolved because this seemed
> interesting.

Welcome! Other opinions on such a topic are always appreciated.

> On Thursday, 12 June 2025 11:17:52 Central European Summer Time Pavel Machek wrote:
> >
> > > Jonathan mentioned recently that he would like to get away from using
> > > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > > have it on good authority that initializing a struct or array with = { }
> > > is the preferred way to do this in the kernel [1]. So here is a series
> > > to take care of that.
> >
> > 1) Is it worth the churn?
> >
> > 2) Will this fail to initialize padding with some obscure compiler?
>
> as of right now, the only two C compilers that are supported are
> GCC >= 8.1, and Clang >= 13.0.1. If anyone even manages to get the kernel
> to finish a build with something else, I think the compiler not
> implementing the C standard correctly is the least of their worries.
>
> My bigger worry is that = { } is only guaranteed to be as correct as
> memset on C23, and the kernel's standard right now is C11. For that
> reason alone, I don't think memset should be moved away from for now,
> unless someone can verify that every GCC release >= 8.1 and every
> Clang release >= 13.0.1 does the right thing here regardless.
>
> >
> > 3) Why do you believe that {} is the preffered way? All we have is
> > Kees' email that explains that = {} maybe works in configs he tested.
>
> = { } is guaranteed to work in C23, as per the standard, but again we're
> not on C23.
>
> The reason to prefer this is likely that it's easier for static analysis
> to see the struct as initialised, but that's me making assumptions here.
>
> A more human-centric argument is that once we're on a C standards version
> where = { } is guaranteed to be correct, then = { } is much more obviously
> correct to a reader than a memset with a value and a size somewhere later
> in the code. This argument is evident from the number of patches in this
> series where the memset and the declaration are not in the same hunk.
> That's the kind of stuff that keeps me awake at night, sweating profusely.

While all you said seems true and I agree with, the pedantism here is
not needed as in the Linux kernel we have {} used for ages in tons of
code and if something went wrong with that we should have had bug
reports already. Are you aware of such? Personally I haven't heard
even one related to this. But if you know, I am really more than
interested to read about (please, give pointers to such a discussion).

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Jonathan Cameron 4 months ago
On Thu, 12 Jun 2025 11:17:52 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > Jonathan mentioned recently that he would like to get away from using
> > memset() to zero-initialize stack memory in the IIO subsystem. And we
> > have it on good authority that initializing a struct or array with = { }
> > is the preferred way to do this in the kernel [1]. So here is a series
> > to take care of that.  
> 
> 1) Is it worth the churn?
> 
> 2) Will this fail to initialize padding with some obscure compiler?
> 
> 3) Why do you believe that {} is the preffered way? All we have is
> Kees' email that explains that = {} maybe works in configs he tested.
> 
Pavel,

I think main thing that matters in Kees email is there is a self test
that should fire if a compiler ever does this wrong.

Using this syntax is definitely not a 'kernel wide' preference yet
but I do prefer to make some changes like this in IIO just because it
reduces the amount of code that smells different when reviewing.
Given how many drivers we now have, sadly people pick different ones
to cut and paste from so we get a lot of new drivers that look like
how we preferred to do things 10 years ago :(

However it is a fair bit of churn. hmm.  Let's let this sit for
a little while and see if other view points come in.

Thanks 

Jonathan
 

> BR,
> 								Pavel
> 
> > [1]:
> > https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/  
> 
> 
> 
> > ---
> > David Lechner (28):
> >       iio: accel: adxl372: use = { } instead of memset()
> >       iio: accel: msa311: use = { } instead of memset()
> >       iio: adc: dln2-adc: use = { } instead of memset()
> >       iio: adc: mt6360-adc: use = { } instead of memset()
> >       iio: adc: rockchip_saradc: use = { } instead of memset()
> >       iio: adc: rtq6056: use = { } instead of memset()
> >       iio: adc: stm32-adc: use = { } instead of memset()
> >       iio: adc: ti-ads1015: use = { } instead of memset()
> >       iio: adc: ti-ads1119: use = { } instead of memset()
> >       iio: adc: ti-lmp92064: use = { } instead of memset()
> >       iio: adc: ti-tsc2046: use = { } instead of memset()
> >       iio: chemical: scd4x: use = { } instead of memset()
> >       iio: chemical: scd30: use = { } instead of memset()
> >       iio: chemical: sunrise_co2: use = { } instead of memset()
> >       iio: dac: ad3552r: use = { } instead of memset()
> >       iio: imu: inv_icm42600: use = { } instead of memset()
> >       iio: imu: inv_mpu6050: use = { } instead of memset()
> >       iio: light: bh1745: use = { } instead of memset()
> >       iio: light: ltr501: use = { } instead of memset()
> >       iio: light: opt4060: use = { } instead of memset()
> >       iio: light: veml6030: use = { } instead of memset()
> >       iio: magnetometer: af8133j: use = { } instead of memset()
> >       iio: pressure: bmp280: use = { } instead of memset()
> >       iio: pressure: mpl3115: use = { } instead of memset()
> >       iio: pressure: mprls0025pa: use = { } instead of memset()
> >       iio: pressure: zpa2326: use = { } instead of memset()
> >       iio: proximity: irsd200: use = { } instead of memset()
> >       iio: temperature: tmp006: use = { } instead of memset()
> > 
> >  drivers/iio/accel/adxl372.c                       | 3 +--
> >  drivers/iio/accel/msa311.c                        | 4 +---
> >  drivers/iio/adc/dln2-adc.c                        | 4 +---
> >  drivers/iio/adc/mt6360-adc.c                      | 3 +--
> >  drivers/iio/adc/rockchip_saradc.c                 | 4 +---
> >  drivers/iio/adc/rtq6056.c                         | 4 +---
> >  drivers/iio/adc/stm32-adc.c                       | 3 +--
> >  drivers/iio/adc/ti-ads1015.c                      | 4 +---
> >  drivers/iio/adc/ti-ads1119.c                      | 4 +---
> >  drivers/iio/adc/ti-lmp92064.c                     | 4 +---
> >  drivers/iio/adc/ti-tsc2046.c                      | 3 +--
> >  drivers/iio/chemical/scd30_core.c                 | 3 +--
> >  drivers/iio/chemical/scd4x.c                      | 3 +--
> >  drivers/iio/chemical/sunrise_co2.c                | 6 ++----
> >  drivers/iio/dac/ad3552r.c                         | 3 +--
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
> >  drivers/iio/light/bh1745.c                        | 4 +---
> >  drivers/iio/light/ltr501.c                        | 4 +---
> >  drivers/iio/light/opt4060.c                       | 4 +---
> >  drivers/iio/light/veml6030.c                      | 4 +---
> >  drivers/iio/magnetometer/af8133j.c                | 4 +---
> >  drivers/iio/pressure/bmp280-core.c                | 5 +----
> >  drivers/iio/pressure/mpl3115.c                    | 3 +--
> >  drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
> >  drivers/iio/pressure/zpa2326.c                    | 4 +---
> >  drivers/iio/proximity/irsd200.c                   | 3 +--
> >  drivers/iio/temperature/tmp006.c                  | 4 +---
> >  30 files changed, 34 insertions(+), 85 deletions(-)
> > ---
> > base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
> > change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb
> > 
> > Best regards,  
>
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Andy Shevchenko 4 months ago
On Wed, Jun 11, 2025 at 05:38:52PM -0500, David Lechner wrote:
> Jonathan mentioned recently that he would like to get away from using
> memset() to zero-initialize stack memory in the IIO subsystem. And we
> have it on good authority that initializing a struct or array with = { }
> is the preferred way to do this in the kernel [1]. So here is a series
> to take care of that.

I believe we may do that independently of the compilers that can leave garbage
in the padding. In case it happens, it will be good adventure to fix the bugs
in the code, that for some reason take padding(s) into account for the real
values.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 00/28] iio: zero init stack with { } instead of memset()
Posted by Nuno Sá 4 months ago
On Wed, 2025-06-11 at 17:38 -0500, David Lechner wrote:
> Jonathan mentioned recently that he would like to get away from using
> memset() to zero-initialize stack memory in the IIO subsystem. And we
> have it on good authority that initializing a struct or array with = { }
> is the preferred way to do this in the kernel [1]. So here is a series
> to take care of that.
> 
> [1]: https://lore.kernel.org/linux-iio/202505090942.48EBF01B@keescook/
> 
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> David Lechner (28):
>       iio: accel: adxl372: use = { } instead of memset()
>       iio: accel: msa311: use = { } instead of memset()
>       iio: adc: dln2-adc: use = { } instead of memset()
>       iio: adc: mt6360-adc: use = { } instead of memset()
>       iio: adc: rockchip_saradc: use = { } instead of memset()
>       iio: adc: rtq6056: use = { } instead of memset()
>       iio: adc: stm32-adc: use = { } instead of memset()
>       iio: adc: ti-ads1015: use = { } instead of memset()
>       iio: adc: ti-ads1119: use = { } instead of memset()
>       iio: adc: ti-lmp92064: use = { } instead of memset()
>       iio: adc: ti-tsc2046: use = { } instead of memset()
>       iio: chemical: scd4x: use = { } instead of memset()
>       iio: chemical: scd30: use = { } instead of memset()
>       iio: chemical: sunrise_co2: use = { } instead of memset()
>       iio: dac: ad3552r: use = { } instead of memset()
>       iio: imu: inv_icm42600: use = { } instead of memset()
>       iio: imu: inv_mpu6050: use = { } instead of memset()
>       iio: light: bh1745: use = { } instead of memset()
>       iio: light: ltr501: use = { } instead of memset()
>       iio: light: opt4060: use = { } instead of memset()
>       iio: light: veml6030: use = { } instead of memset()
>       iio: magnetometer: af8133j: use = { } instead of memset()
>       iio: pressure: bmp280: use = { } instead of memset()
>       iio: pressure: mpl3115: use = { } instead of memset()
>       iio: pressure: mprls0025pa: use = { } instead of memset()
>       iio: pressure: zpa2326: use = { } instead of memset()
>       iio: proximity: irsd200: use = { } instead of memset()
>       iio: temperature: tmp006: use = { } instead of memset()
> 
>  drivers/iio/accel/adxl372.c                       | 3 +--
>  drivers/iio/accel/msa311.c                        | 4 +---
>  drivers/iio/adc/dln2-adc.c                        | 4 +---
>  drivers/iio/adc/mt6360-adc.c                      | 3 +--
>  drivers/iio/adc/rockchip_saradc.c                 | 4 +---
>  drivers/iio/adc/rtq6056.c                         | 4 +---
>  drivers/iio/adc/stm32-adc.c                       | 3 +--
>  drivers/iio/adc/ti-ads1015.c                      | 4 +---
>  drivers/iio/adc/ti-ads1119.c                      | 4 +---
>  drivers/iio/adc/ti-lmp92064.c                     | 4 +---
>  drivers/iio/adc/ti-tsc2046.c                      | 3 +--
>  drivers/iio/chemical/scd30_core.c                 | 3 +--
>  drivers/iio/chemical/scd4x.c                      | 3 +--
>  drivers/iio/chemical/sunrise_co2.c                | 6 ++----
>  drivers/iio/dac/ad3552r.c                         | 3 +--
>  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 5 ++---
>  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 5 ++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c        | 4 +---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c        | 6 ++----
>  drivers/iio/light/bh1745.c                        | 4 +---
>  drivers/iio/light/ltr501.c                        | 4 +---
>  drivers/iio/light/opt4060.c                       | 4 +---
>  drivers/iio/light/veml6030.c                      | 4 +---
>  drivers/iio/magnetometer/af8133j.c                | 4 +---
>  drivers/iio/pressure/bmp280-core.c                | 5 +----
>  drivers/iio/pressure/mpl3115.c                    | 3 +--
>  drivers/iio/pressure/mprls0025pa_i2c.c            | 5 +----
>  drivers/iio/pressure/zpa2326.c                    | 4 +---
>  drivers/iio/proximity/irsd200.c                   | 3 +--
>  drivers/iio/temperature/tmp006.c                  | 4 +---
>  30 files changed, 34 insertions(+), 85 deletions(-)
> ---
> base-commit: 4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
> change-id: 20250611-iio-zero-init-stack-with-instead-of-memset-0d12d41a7ecb
> 
> Best regards,