Re: [PATCH] iio: proximity: aw96103: validate firmware data length

Jonathan Cameron <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Message-ID <20260802192134.32a0349d@jic23-huawei>
On Sun,  2 Aug 2026 01:22:00 +0545
Laxman Acharya Padhya <[email protected]> wrote:

> The firmware parser reads a length from the binary header and then uses
> it to walk six-byte register/value entries. A truncated firmware image
> can make the parser read beyond the copied firmware buffer. A value
> smaller than the four-byte count field also underflows, producing a very
> large length.
> 
> Read the little-endian field with get_unaligned_le32(), as previously
> proposed by David Lechner. Validate the header size, reject an
> underflowed length, ensure the resulting payload is contained in the
> firmware image, and require it to contain whole register/value entries.
> 
> Fixes: 07b241262dca ("iio: proximity: aw96103: Add support for aw96103/aw96105 proximity sensor")
> Link: https://lore.kernel.org/r/20260314-iio-proximity-aw96103-fix-firmware-read-v1-1-c74fe9ccd82b@baylibre.com
> Cc: [email protected]
> Signed-off-by: Laxman Acharya Padhya <[email protected]>
Looks fine to me, but I'll leave it on list a little while so others
have time to take a look, ideally including Shuaijie Wang

Thanks

Jonathan

> ---
>  drivers/iio/proximity/aw96103.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/proximity/aw96103.c b/drivers/iio/proximity/aw96103.c
> index 8fbb755dc..8352d51e5 100644
> --- a/drivers/iio/proximity/aw96103.c
> +++ b/drivers/iio/proximity/aw96103.c
> @@ -24,6 +24,7 @@
>  #define AW96103_BIN_VALID_DATA_OFFSET		64
>  #define AW96103_BIN_DATA_LEN_OFFSET		16
>  #define AW96103_BIN_DATA_REG_NUM_SIZE		4
> +#define AW96103_BIN_REG_SIZE			6
>  #define AW96103_BIN_CHIP_TYPE_SIZE		8
>  #define AW96103_BIN_CHIP_TYPE_OFFSET		24
>  
> @@ -229,14 +230,27 @@ static const struct aw_chip_info aw_chip_info_tbl[] = {
>  	},
>  };
>  
> -static void aw96103_parsing_bin_file(struct aw_bin *bin)
> +static int aw96103_parsing_bin_file(struct aw_bin *bin)
>  {
> +	u32 data_len;
> +
> +	if (bin->len < AW96103_BIN_VALID_DATA_OFFSET)
> +		return -EINVAL;
> +
> +	data_len = get_unaligned_le32(bin->data + AW96103_BIN_DATA_LEN_OFFSET);
> +	if (data_len < AW96103_BIN_DATA_REG_NUM_SIZE)
> +		return -EINVAL;
> +
> +	bin->valid_data_len = data_len - AW96103_BIN_DATA_REG_NUM_SIZE;
> +	if (bin->valid_data_len > bin->len - AW96103_BIN_VALID_DATA_OFFSET ||
> +	    bin->valid_data_len % AW96103_BIN_REG_SIZE)
> +		return -EINVAL;
> +
>  	bin->valid_data_addr = AW96103_BIN_VALID_DATA_OFFSET;
> -	bin->valid_data_len =
> -		*(unsigned int *)(bin->data + AW96103_BIN_DATA_LEN_OFFSET) -
> -		AW96103_BIN_DATA_REG_NUM_SIZE;
>  	memcpy(bin->chip_type, bin->data + AW96103_BIN_CHIP_TYPE_OFFSET,
>  	       AW96103_BIN_CHIP_TYPE_SIZE);
> +
> +	return 0;
>  }
>  
>  static const struct regmap_config aw96103_regmap_confg = {
> @@ -500,7 +514,7 @@ static int aw96103_bin_valid_loaded(struct aw96103 *aw96103,
>  	int ret;
>  
>  	for (i = 0; i < aw_bin_data_s->valid_data_len;
> -	     i += 6, start_addr += 6) {
> +	     i += AW96103_BIN_REG_SIZE, start_addr += AW96103_BIN_REG_SIZE) {
>  		reg_addr = get_unaligned_le16(aw_bin_data_s->data + start_addr);
>  		reg_data = get_unaligned_le32(aw_bin_data_s->data +
>  					      start_addr + 2);
> @@ -550,6 +564,8 @@ static int aw96103_para_loaded(struct aw96103 *aw96103)
>  static int aw96103_cfg_all_loaded(const struct firmware *cont,
>  				  struct aw96103 *aw96103)
>  {
> +	int ret;
> +
>  	if (!cont)
>  		return -EINVAL;
>  
> @@ -561,7 +577,9 @@ static int aw96103_cfg_all_loaded(const struct firmware *cont,
>  	aw_bin->len = cont->size;
>  	memcpy(aw_bin->data, cont->data, cont->size);
>  	release_firmware(cont);
> -	aw96103_parsing_bin_file(aw_bin);
> +	ret = aw96103_parsing_bin_file(aw_bin);
> +	if (ret)
> +		return ret;
>  
>  	return aw96103_bin_valid_loaded(aw96103, aw_bin);
>  }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.