[PATCH] pwm: lp3943: validate DT output indices in lp3943_pwm_parse_dt()

[PATCH] pwm: lp3943: validate DT output indices in lp3943_pwm_parse_dt()

[Manush Prajwal]

The ti,pwm0/ti,pwm1 devicetree properties are raw u32 arrays naming
which of the 16 physical LP3943 outputs (0-15, per the documented
binding and the 16-entry enum lp3943_pwm_output) each PWM channel
drives. lp3943_pwm_parse_dt() reads these values straight from the
devicetree with of_property_read_u32_array() and stores them in
pwm_map->output[] without checking they are actually in range.

Both consumers of pwm_map->output[] then use an out-of-range value
directly as an index/bit-offset with no bounds check of their own:

 - lp3943_pwm_request_map() does
       test_and_set_bit(offset, &lp3943->pin_used)
   with offset taken straight from output[i], corrupting memory beyond
   the single 'unsigned long pin_used' field for offset >= BITS_PER_LONG.

 - lp3943_pwm_set_mode() does
       mux[index].reg / .mask / .shift
   with index taken straight from output[i], reading past the 16-entry
   lp3943_mux_cfg[] array and then issuing a regmap write built from
   whatever garbage register/mask/shift values were read out of bounds.

A devicetree (malformed board file or a loaded overlay) with an
out-of-range ti,pwm0/ti,pwm1 value is enough to reach either path;
nothing upstream of these two functions validates the value.

Add a range check in lp3943_pwm_parse_dt() right after the values are
read, rejecting the devicetree with -EINVAL before any out-of-range
value can reach pwm_map->output[]. Add the LP3943_NUM_OUTPUTS(16)
constant next to the existing enum it bounds, alongside the existing
LP3943_NUM_PWMS(2) constant.

Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
---
 drivers/pwm/pwm-lp3943.c   | 7 ++++++-
 include/linux/mfd/lp3943.h | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-lp3943.c b/drivers/pwm/pwm-lp3943.c
index 10537e74b..1ce2a1bbc 100644
--- a/drivers/pwm/pwm-lp3943.c
+++ b/drivers/pwm/pwm-lp3943.c
@@ -218,7 +218,7 @@ static int lp3943_pwm_parse_dt(struct device *dev,
 	struct lp3943_platform_data *pdata;
 	struct lp3943_pwm_map *pwm_map;
 	enum lp3943_pwm_output *output;
-	int i, err, num_outputs, count = 0;
+	int i, j, err, num_outputs, count = 0;
 
 	if (!node)
 		return -EINVAL;
@@ -247,6 +247,11 @@ static int lp3943_pwm_parse_dt(struct device *dev,
 		if (err)
 			return err;
 
+		for (j = 0; j < num_outputs; j++) {
+			if (output[j] >= LP3943_NUM_OUTPUTS)
+				return -EINVAL;
+		}
+
 		pwm_map = devm_kzalloc(dev, sizeof(*pwm_map), GFP_KERNEL);
 		if (!pwm_map)
 			return -ENOMEM;
diff --git a/include/linux/mfd/lp3943.h b/include/linux/mfd/lp3943.h
index 5d2d172d3..844938ca7 100644
--- a/include/linux/mfd/lp3943.h
+++ b/include/linux/mfd/lp3943.h
@@ -53,6 +53,8 @@ enum lp3943_pwm_output {
 	LP3943_PWM_OUT15,
 };
 
+#define LP3943_NUM_OUTPUTS		16
+
 /*
  * struct lp3943_pwm_map
  * @output: Output pins which are mapped to each PWM channel
-- 
2.46.2.windows.1

Re: [PATCH] pwm: lp3943: validate DT output indices in lp3943_pwm_parse_dt()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added bounds check for device tree output indices is incomplete because it performs a signed comparison. Negative values can bypass the check, leading to out-of-bounds memory accesses.

Pre-existing issues:
- [High] Missing NULL check on `pdata->pwms[hwpwm]` leads to a crash if a partially configured PWM chip is requested.
--

commit 6bf884f857d8ed9b87f551fe2b3bdb8a985eae4f
Author: Manush Prajwal <manushprajwal555@gmail.com>

pwm: lp3943: validate DT output indices in lp3943_pwm_parse_dt()

This commit adds a bounds check in lp3943_pwm_parse_dt() to validate PWM
output indices read from the devicetree. It ensures that the parsed output
values are within the valid range before storing them.

> diff --git a/drivers/pwm/pwm-lp3943.c b/drivers/pwm/pwm-lp3943.c
> index 10537e74be288..1ce2a1bbc5fd5 100644
> --- a/drivers/pwm/pwm-lp3943.c
> +++ b/drivers/pwm/pwm-lp3943.c

[ ... ]

> @@ -247,6 +247,11 @@ static int lp3943_pwm_parse_dt(struct device *dev,
>  		if (err)
>  			return err;
>  
> +		for (j = 0; j < num_outputs; j++) {
> +			if (output[j] >= LP3943_NUM_OUTPUTS)
> +				return -EINVAL;
> +		}
> +

[Severity: High]
Does this bounds check fail to reject negative values resulting from type
conversion, leaving the driver vulnerable to out-of-bounds memory accesses?

A malformed devicetree (e.g., loaded via DT overlays) can provide a large
32-bit value with the MSB set (like 0xFFFFFFFF) for a PWM output index.

Because enum lp3943_pwm_output is treated as a signed integer type under
standard ABIs, the value is interpreted as negative.

The newly added check if (output[j] >= LP3943_NUM_OUTPUTS) performs a signed
comparison because LP3943_NUM_OUTPUTS (16) is a signed literal. The negative
value evaluates as less than 16, incorrectly bypassing the bounds check.

The negative value can then be used as an out-of-bounds array index:

drivers/pwm/pwm-lp3943.c:lp3943_pwm_set_mode() {
    ...
		index = pwm_map->output[i];
		err = lp3943_update_bits(lp3943, mux[index].reg,
					 mux[index].mask,
					 val << mux[index].shift);
    ...
}

>  		pwm_map = devm_kzalloc(dev, sizeof(*pwm_map), GFP_KERNEL);
>  		if (!pwm_map)
>  			return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a missing
NULL check on pdata->pwms[hwpwm] to lead to a crash if a partially configured
PWM chip is requested?

During probe, lp3943_pwm_parse_dt() iterates over the ti,pwm0 and ti,pwm1
properties. If a property is missing, the loop skips initializing that channel,
leaving pdata->pwms[i] as NULL.

However, the probe function registers the PWM chip unconditionally for both
channels. If a sysfs user requests the missing channel, the PWM core invokes
lp3943_pwm_request_map(), which blindly dereferences pdata->pwms[hwpwm]:

drivers/pwm/pwm-lp3943.c:lp3943_pwm_request_map() {
    ...
	pwm_map->output = pdata->pwms[hwpwm]->output;
	pwm_map->num_outputs = pdata->pwms[hwpwm]->num_outputs;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6a9d4c6b.79b5ea6e.147aa1.5883@mx.google.com?part=1