Re: [PATCH] iio: st_sensors: add __counted_by_ptr to struct st_sensor_settings

[Kees Cook <kees@kernel.org>]

On Tue, Sep 22, 2026 at 11:21:11AM +0000, Bill Wendling wrote:
> Annotate the "ch" pointer member of "struct st_sensor_settings" with the
> "__counted_by_ptr" attribute. The elements of "ch" are counted by the
> "num_ch" member in the same struct.
> 
> All instances of "struct st_sensor_settings" are defined as "static
> const" arrays across the ST sensor core drivers. For pressure sensors,
> "num_ch" is explicitly initialized with the size of the respective
> channel array using "ARRAY_SIZE(...)" during static definition.

I wanted to understand this so, in drivers/iio/accel/st_accel_core.c:

static const struct st_sensor_settings st_accel_sensors_settings[] = {
...
                .ch = (struct iio_chan_spec *)st_accel_12bit_channels,
...
}

static const struct iio_chan_spec st_accel_12bit_channels[] = {
        ST_SENSORS_LSM_CHANNELS_EXT(IIO_ACCEL,
                        BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
                        ST_SENSORS_SCAN_X, 1, IIO_MOD_X, 's', IIO_LE, 12, 16,
                        ST_ACCEL_DEFAULT_OUT_X_L_ADDR,
                        st_accel_mount_matrix_ext_info),
	...
        IIO_CHAN_SOFT_TIMESTAMP(3)
};

#define IIO_CHAN_SOFT_TIMESTAMP(_si) (struct iio_chan_spec) {           \
        .type = IIO_TIMESTAMP,                                          \
        .channel = -1,                                                  \
        .scan_index = _si,                                              \
        .scan_type = {                                                  \
                .sign = 's',                                            \
                .realbits = 64,                                         \
                .storagebits = 64,                                      \
        },                                                              \
}

So .ch is assigned an array of items which looks to end with a termination
sentinel. And .num_ch is unassigned:

$ git grep '\.ch = ' drivers/iio/accel/st_accel_core.c | wc -l
16
$ git grep '\.num_ch = ' drivers/iio/accel/st_accel_core.c | wc -l
0

> For accelerometer, gyroscope, and magnetometer sensors, "num_ch" is not
> explicitly initialized (and thus defaults to 0). This is because those
> drivers hardcode the channel count to "ST_SENSORS_NUMBER_ALL_CHANNELS"
> rather than using "num_ch" from the settings struct.

Yeah, checks out:

$ git grep '\.ch = ' | cut -d: -f1 | sort | uniq -c
     16 accel/st_accel_core.c
      4 gyro/st_gyro_core.c
      5 magnetometer/st_magn_core.c
      6 pressure/st_pressure_core.c
$ git grep '\.num_ch = ' | cut -d: -f1 | sort | uniq -c
      6 pressure/st_pressure_core.c

Interestingly, the pressure/st_pressure_core.c uses both assigned .num_ch
_and_ a sentinel.

> Since these structures are static const, both "ch" and "num_ch" are
> fully initialized at compile time and available immediately at boot
> time.  The only accesses to the "ch" field of "st_sensor_settings" occur
> when assigning it to "indio_dev->channels" during device probing.

Heh, so the bounds check gets laundered. :) Even pressure:

pressure/st_pressure_core.c:    press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
pressure/st_pressure_core.c:    indio_dev->num_channels = press_data->sensor_settings->num_ch;

> Because "sensor_settings->ch" is never dereferenced or accessed as an
> array, adding the "__counted_by_ptr" annotation does not cause any
> runtime panics or false-positive bounds checks under KASAN or UBSAN.

I would argue that either __counted_by_ptr has no use here ("accidentally
safe because no one dereferences .ch" isn't a great justification)
or that the accelerometer, gyroscope, and magnetometer sensors should
have their .num_ch assigned correctly so that the "contract" is correct,
even if nothing does the deref.

-Kees

-- 
Kees Cook