Re: [PATCH] iio: imu: st_lsm6dsx: Fix reading array out of bounds

[Alexandre Bard <alexandre.bard@netmodule.com>]

Le 09.04.20 à 13:01, Stephan Gerhold a écrit :
> On Thu, Apr 09, 2020 at 10:58:18AM +0200, Alexandre Bard wrote:
>> Former code was iterating through all possible IDs whereas only a few
>> per settings array are really available. Leading to several out of
>> bounds readings.
>>
>> Line is now longer than 80 characters. But since it is a classic for
>> loop I think it is better to keep it like this than splitting it.
>>
>> Signed-off-by: Alexandre Bard <alexandre.bard@netmodule.com>
>> ---
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index 84d219ae6aee..be8882ff30eb 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -1350,7 +1350,7 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
>>  	int err, i, j, data;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
>> -		for (j = 0; j < ST_LSM6DSX_MAX_ID; j++) {
>> +		for (j = 0; j < ARRAY_SIZE(st_lsm6dsx_sensor_settings[i].id); j++) {
> id in st_lsm6dsx_settings is declared as:
>
> 	struct {
> 		enum st_lsm6dsx_hw_id hw_id;
> 		const char *name;
> 	} id[ST_LSM6DSX_MAX_ID];
>
> so it's always ST_LSM6DSX_MAX_ID long
> (additional entries are just zero-initialized).
>
> Isn't ARRAY_SIZE(st_lsm6dsx_sensor_settings[i].id) == ST_LSM6DSX_MAX_ID
> in this case?
Yes, you are right, I missed that. But there is still a problem :
parsing 0-initialized fields can lead to a false positive when looking for the
value ST_LSM6DS3_ID which is the first element of an enum. So either the enum
must be patched to start at 1 or the length of valid ids in a settings must be
retrieved somehow.

Or is there another way ? Or am I wrong ?
>
>>  			if (st_lsm6dsx_sensor_settings[i].id[j].name &&
>>  			    id == st_lsm6dsx_sensor_settings[i].id[j].hw_id)
>>  				break;
>> -- 
>> 2.20.1
>>