[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

[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>

---
v2:
- Explain the U16_MAX limit in a code comment.
- Keep the existing kzalloc() expression unchanged.

---
 drivers/iio/proximity/hx9023s.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
index a6ff7cbe9e65..685053b84b34 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,6 +1032,10 @@ 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)
 {
+ /* fw_size is u16 in struct hx9023s_bin, so reject truncation. */
+ 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);
  if (!bin)
@@ -1041,7 +1046,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 +1064,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 Sat, Jul 11, 2026 at 08:28:27AM +0545, 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.

You send the same mail twice without any word saying why. On top of that both
times we got the mangled patch. Please, slow down and check first your email
and other configurations, send to yourself (perhaps different email address)
to check how the result will look like.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Sat, Jul 11, 2026 at 08:25:58AM +0545, 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.

> Fixes: e9ed97be4fcc ("iio: proximity: hx9023s: Added firmware file parsing
> functionality")

Actually the tags are supposed to be "one (full) tag per line". In the second
message it seems incorrectly wrapped.

Also see my reply to the other email of yours.

-- 
With Best Regards,
Andy Shevchenko