On Thu, 9 Sep 2021 12:13:36 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> The "idx" is validated at the start of the loop but it gets incremented
> during the iteration so it needs to be checked again.
>
> Fixes: 50dd64d57eee ("iio: common: ssp_sensors: Add sensorhub driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
This is only a fix if we assume that the len value check is there
as a protection against buffer overrun rather than as a termination condition
that occurs when parsing a valid record.
There is more paranoid checking in ssp_print_mc_debug() so it seems we aren't assuming
valid data in there at least.
Still is this perhaps a case of hardening rather than a fix or am I missing something?
As an aside, if that ssp_print_mcu_debug() reads a negative char it is then
returned directly so we get a random small negative number as the error code which
isn't going to be very useful.
Jonathan
> ---
> drivers/iio/common/ssp_sensors/ssp_spi.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
> index 4864c38b8d1c..387551eac184 100644
> --- a/drivers/iio/common/ssp_sensors/ssp_spi.c
> +++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
> @@ -273,6 +273,8 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
> for (idx = 0; idx < len;) {
> switch (dataframe[idx++]) {
> case SSP_MSG2AP_INST_BYPASS_DATA:
> + if (idx >= len)
> + return -EPROTO;
> sd = dataframe[idx++];
> if (sd < 0 || sd >= SSP_SENSOR_MAX) {
> dev_err(SSP_DEV,
> @@ -282,10 +284,13 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
>
> if (indio_devs[sd]) {
> spd = iio_priv(indio_devs[sd]);
> - if (spd->process_data)
> + if (spd->process_data) {
> + if (idx >= len)
> + return -EPROTO;
> spd->process_data(indio_devs[sd],
> &dataframe[idx],
> data->timestamp);
> + }
> } else {
> dev_err(SSP_DEV, "no client for frame\n");
> }
> @@ -293,6 +298,8 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
> idx += ssp_offset_map[sd];
> break;
> case SSP_MSG2AP_INST_DEBUG_DATA:
> + if (idx >= len)
> + return -EPROTO;
> sd = ssp_print_mcu_debug(dataframe, &idx, len);
> if (sd) {
> dev_err(SSP_DEV,
