[PATCH] iio: proximity: hx9023s: validate firmware size

[PATCH] iio: proximity: hx9023s: validate firmware size

[Laxman Acharya Padhya]

hx9023s_send_cfg() copies the firmware into a counted flexible array and
then reads fixed offsets from the copied data before walking register/value
pairs starting at FW_DATA_OFFSET. A truncated firmware image can therefore
make the driver read past the copied buffer during probe-time configuration
loading.

Reject firmware images that cannot contain the fixed header, reject images
too large for the u16 fw_size field, and validate that the advertised
register count fits in the remaining payload.

Move release_firmware() to the callback so the firmware object is released
on all hx9023s_send_cfg() error paths.

Fixes: e9ed97be4fcc ("iio: proximity: hx9023s: Added firmware file parsing functionality")
Cc: stable@vger.kernel.org
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
 drivers/iio/proximity/hx9023s.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
index a6ff7cbe9e6..a2f9c077e58 100644
--- a/drivers/iio/proximity/hx9023s.c
+++ b/drivers/iio/proximity/hx9023s.c
@@ -18,6 +18,7 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
+#include <linux/limits.h>
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -25,6 +26,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/units.h>
 
@@ -1031,8 +1033,12 @@ static int hx9023s_bin_load(struct hx9023s_data *data, struct hx9023s_bin *bin)
 
 static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data)
 {
-	struct hx9023s_bin *bin __free(kfree) =
-		kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
+	struct hx9023s_bin *bin __free(kfree) = NULL;
+
+	if (fw->size < FW_DATA_OFFSET || fw->size > U16_MAX)
+		return -EINVAL;
+
+	bin = kzalloc(sizeof(*bin) + fw->size, GFP_KERNEL);
 	if (!bin)
 		return -ENOMEM;
 
@@ -1041,7 +1047,8 @@ static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data
 	bin->fw_ver = bin->data[FW_VER_OFFSET];
 	bin->reg_count = get_unaligned_le16(bin->data + FW_REG_CNT_OFFSET);
 
-	release_firmware(fw);
+	if (bin->reg_count > (bin->fw_size - FW_DATA_OFFSET) / 2)
+		return -EINVAL;
 
 	return hx9023s_bin_load(data, bin);
 }
@@ -1058,6 +1065,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
 	}
 
 	ret = hx9023s_send_cfg(fw, data);
+	release_firmware(fw);
 	if (ret) {
 		dev_warn(dev, "Firmware update failed: %d\n", ret);
 		goto no_fw;

base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
-- 
2.51.2

Re: [PATCH] iio: proximity: hx9023s: validate firmware size

[Joshua Crofts]

On Fri, 10 Jul 2026 20:07:12 +0545
Laxman Acharya Padhya <acharyalaxman8848@gmail.com> wrote:
> @@ -1058,6 +1065,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
>  	}
>  
>  	ret = hx9023s_send_cfg(fw, data);
> +	release_firmware(fw);

Why not move this after the if? Keep the call/retval check coupled.

>  	if (ret) {
>  		dev_warn(dev, "Firmware update failed: %d\n", ret);
>  		goto no_fw;
> 

Otherwise this looks good. Feel free to add my tag with the v2:

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards

CJD

Re: [PATCH] iio: proximity: hx9023s: validate firmware size

[David Lechner]

On 7/10/26 9:54 AM, Joshua Crofts wrote:
> On Fri, 10 Jul 2026 20:07:12 +0545
> Laxman Acharya Padhya <acharyalaxman8848@gmail.com> wrote:
>> @@ -1058,6 +1065,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
>>  	}
>>  
>>  	ret = hx9023s_send_cfg(fw, data);
>> +	release_firmware(fw);
> 
> Why not move this after the if? Keep the call/retval check coupled.

Because the if branch contains a goto and would therefore skip calling
release_firmware() in case of error.

> 
>>  	if (ret) {
>>  		dev_warn(dev, "Firmware update failed: %d\n", ret);
>>  		goto no_fw;
>>
> 
> Otherwise this looks good. Feel free to add my tag with the v2:
> 
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> 

Re: [PATCH] iio: proximity: hx9023s: validate firmware size

[David Lechner]

On 7/10/26 9:22 AM, Laxman Acharya Padhya wrote:
> hx9023s_send_cfg() copies the firmware into a counted flexible array and
> then reads fixed offsets from the copied data before walking register/value
> pairs starting at FW_DATA_OFFSET. A truncated firmware image can therefore
> make the driver read past the copied buffer during probe-time configuration
> loading.
> 
> Reject firmware images that cannot contain the fixed header, reject images
> too large for the u16 fw_size field, and validate that the advertised
> register count fits in the remaining payload.
> 
> Move release_firmware() to the callback so the firmware object is released
> on all hx9023s_send_cfg() error paths.

This could probably be considered a separate fix (and therefore separate
patch) since it looks like there is an existing code path (return -ENOMEM)
where it would not be released.

> 
> Fixes: e9ed97be4fcc ("iio: proximity: hx9023s: Added firmware file parsing functionality")
> Cc: stable@vger.kernel.org
> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
> ---
>  drivers/iio/proximity/hx9023s.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> index a6ff7cbe9e6..a2f9c077e58 100644
> --- a/drivers/iio/proximity/hx9023s.c
> +++ b/drivers/iio/proximity/hx9023s.c
> @@ -18,6 +18,7 @@
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/irqreturn.h>
> +#include <linux/limits.h>
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -25,6 +26,7 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/units.h>
>  
> @@ -1031,8 +1033,12 @@ static int hx9023s_bin_load(struct hx9023s_data *data, struct hx9023s_bin *bin)
>  
>  static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data)
>  {
> -	struct hx9023s_bin *bin __free(kfree) =
> -		kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
> +	struct hx9023s_bin *bin __free(kfree) = NULL;

We don't initialize autocleanup variables to NULL. So leave this as-is
and just put the size check before it.

> +
> +	if (fw->size < FW_DATA_OFFSET || fw->size > U16_MAX)
> +		return -EINVAL;
> +
> +	bin = kzalloc(sizeof(*bin) + fw->size, GFP_KERNEL);
>  	if (!bin)
>  		return -ENOMEM;
>  
> @@ -1041,7 +1047,8 @@ static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data
>  	bin->fw_ver = bin->data[FW_VER_OFFSET];
>  	bin->reg_count = get_unaligned_le16(bin->data + FW_REG_CNT_OFFSET);
>  
> -	release_firmware(fw);
> +	if (bin->reg_count > (bin->fw_size - FW_DATA_OFFSET) / 2)
> +		return -EINVAL;
>  
>  	return hx9023s_bin_load(data, bin);
>  }
> @@ -1058,6 +1065,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
>  	}
>  
>  	ret = hx9023s_send_cfg(fw, data);
> +	release_firmware(fw);
>  	if (ret) {
>  		dev_warn(dev, "Firmware update failed: %d\n", ret);
>  		goto no_fw;
> 
> base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53

[PATCH v2] iio: proximity: hx9023s: validate firmware size

[Laxman Acharya Padhya]

hx9023s_send_cfg() copies the firmware into a counted flexible array and
then reads fixed offsets from the copied data before walking register/value
pairs starting at FW_DATA_OFFSET. A truncated firmware image can therefore
make the driver read past the copied buffer during probe-time configuration
loading.

Reject firmware images that cannot contain the fixed header, reject images
too large for the u16 fw_size field, and validate that the advertised
register count fits in the remaining payload.

Fixes: e9ed97be4fcc ("iio: proximity: hx9023s: Added firmware file parsing functionality")
Cc: stable@vger.kernel.org
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
 drivers/iio/proximity/hx9023s.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
index a6ff7cbe9e6..9d91ce681ac 100644
--- a/drivers/iio/proximity/hx9023s.c
+++ b/drivers/iio/proximity/hx9023s.c
@@ -18,6 +18,7 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
+#include <linux/limits.h>
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -1031,8 +1032,11 @@ static int hx9023s_bin_load(struct hx9023s_data *data, struct hx9023s_bin *bin)
 
 static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data)
 {
+	if (fw->size < FW_DATA_OFFSET || fw->size > U16_MAX)
+		return -EINVAL;
+
 	struct hx9023s_bin *bin __free(kfree) =
-		kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
+		kzalloc(sizeof(*bin) + fw->size, GFP_KERNEL);
 	if (!bin)
 		return -ENOMEM;
 
@@ -1041,7 +1045,8 @@ static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data
 	bin->fw_ver = bin->data[FW_VER_OFFSET];
 	bin->reg_count = get_unaligned_le16(bin->data + FW_REG_CNT_OFFSET);
 
-	release_firmware(fw);
+	if (bin->reg_count > (bin->fw_size - FW_DATA_OFFSET) / 2)
+		return -EINVAL;
 
 	return hx9023s_bin_load(data, bin);
 }
@@ -1058,6 +1063,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
 	}
 
 	ret = hx9023s_send_cfg(fw, data);
+	release_firmware(fw);
 	if (ret) {
 		dev_warn(dev, "Firmware update failed: %d\n", ret);
 		goto no_fw;
-- 
2.51.2

Re: [PATCH v2] iio: proximity: hx9023s: validate firmware size

[Jonathan Cameron]

On Fri, 10 Jul 2026 21:13:42 +0545
Laxman Acharya Padhya <acharyalaxman8848@gmail.com> wrote:

> hx9023s_send_cfg() copies the firmware into a counted flexible array and
> then reads fixed offsets from the copied data before walking register/value
> pairs starting at FW_DATA_OFFSET. A truncated firmware image can therefore
> make the driver read past the copied buffer during probe-time configuration
> loading.
> 
> Reject firmware images that cannot contain the fixed header, reject images
> too large for the u16 fw_size field, and validate that the advertised
> register count fits in the remaining payload.
> 
> Fixes: e9ed97be4fcc ("iio: proximity: hx9023s: Added firmware file parsing functionality")
> Cc: stable@vger.kernel.org
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
A few minor comments.

Thanks,

Jonathan

> ---
>  drivers/iio/proximity/hx9023s.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> index a6ff7cbe9e6..9d91ce681ac 100644
> --- a/drivers/iio/proximity/hx9023s.c
> +++ b/drivers/iio/proximity/hx9023s.c
> @@ -18,6 +18,7 @@
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/irqreturn.h>
> +#include <linux/limits.h>
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -1031,8 +1032,11 @@ static int hx9023s_bin_load(struct hx9023s_data *data, struct hx9023s_bin *bin)
>  
>  static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data)
>  {
> +	if (fw->size < FW_DATA_OFFSET || fw->size > U16_MAX)

Add a comment on why you've picked that upper limit.

> +		return -EINVAL;
> +
>  	struct hx9023s_bin *bin __free(kfree) =
> -		kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
> +		kzalloc(sizeof(*bin) + fw->size, GFP_KERNEL);

This doesn't belong in the fix given it is just a reorder. Maybe it makes sense
but if it does, separate patch with an explanation of why.

>  	if (!bin)
>  		return -ENOMEM;
>  
> @@ -1041,7 +1045,8 @@ static int hx9023s_send_cfg(const struct firmware *fw, struct hx9023s_data *data
>  	bin->fw_ver = bin->data[FW_VER_OFFSET];
>  	bin->reg_count = get_unaligned_le16(bin->data + FW_REG_CNT_OFFSET);
>  
> -	release_firmware(fw);
> +	if (bin->reg_count > (bin->fw_size - FW_DATA_OFFSET) / 2)
> +		return -EINVAL;
>  
>  	return hx9023s_bin_load(data, bin);
>  }
> @@ -1058,6 +1063,7 @@ static void hx9023s_cfg_update(const struct firmware *fw, void *context)
>  	}
>  
>  	ret = hx9023s_send_cfg(fw, data);
> +	release_firmware(fw);
>  	if (ret) {
>  		dev_warn(dev, "Firmware update failed: %d\n", ret);
>  		goto no_fw;

Re: [PATCH v2] iio: proximity: hx9023s: validate firmware size

[Andy Shevchenko]

On Fri, Jul 10, 2026 at 01:21:51PM -0700, Jonathan Cameron wrote:
> On Fri, 10 Jul 2026 21:13:42 +0545
> Laxman Acharya Padhya <acharyalaxman8848@gmail.com> wrote:

...

> >  	struct hx9023s_bin *bin __free(kfree) =
> > -		kzalloc(fw->size + sizeof(*bin), GFP_KERNEL);
> > +		kzalloc(sizeof(*bin) + fw->size, GFP_KERNEL);
> 
> This doesn't belong in the fix given it is just a reorder. Maybe it makes sense
> but if it does, separate patch with an explanation of why.

Semantically this should use struct_size() which brings us to the kzalloc_flex().
And this will be a separate patch.

> >  	if (!bin)
> >  		return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko