Re: [PATCH v2 4/6] mfd: lm3533: Set DMA mask

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `lm3533_cells` array is allocated on the stack without being zero-initialized, and an unconditional `count++` leads to uninitialized elements being passed to the MFD core, risking memory corruption or kernel panics.
- [Medium] An off-by-one error in the validation of the backlight `reg` property allows it to overlap with the LED `reg` address space.
--

commit e3db8e2c46ac0e3ca30554a0b5ae2956e28c6227
Author: Svyatoslav Ryhel <clamor95@gmail.com>

mfd: lm3533: Set DMA mask

Missing coherent_dma_mask assigning triggers a warning in dmesg. This sets
coherent_dma_mask to zero because both the core and cells do not utilize
dma.

> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
> index 519f8c16a3f35..3cfdebf5fb52b 100644
> --- a/drivers/mfd/lm3533-core.c
> +++ b/drivers/mfd/lm3533-core.c
> @@ -526,6 +526,10 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
>  				   LM3533_BOOST_FREQ_MAX);
>  	lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
>  
> +	/* LM3533 and child devices do not use DMA */
> +	i2c->dev.coherent_dma_mask = 0;
> +	i2c->dev.dma_mask = &i2c->dev.coherent_dma_mask;
> +
>  	return lm3533_device_init(lm3533);

[Severity: High]
Does this call to lm3533_device_init() pass uninitialized elements to the mfd
core?  Looking at lm3533_device_init(), the lm3533_cells array is allocated
on the stack without zero-initialization.

If a child node is present in the device tree that does not match any of the
recognized compatible strings, the iteration loop skips the initialization
blocks but still unconditionally executes count++, leaving the array element
with stack garbage:

drivers/mfd/lm3533-core.c:lm3533_device_init() {
    struct mfd_cell lm3533_cells[LM3533_CELLS_MAX];
    u32 count = 0, reg;
    ...
    device_for_each_child_node_scoped(lm3533->dev, child) {
        if (!fwnode_device_is_available(child))
            continue;

        if (count >= LM3533_CELLS_MAX)
            break;

        if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
            ...
        }
        if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
            ...
        }
        if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
            ...
        }

        count++;
    }
    ...
    ret = mfd_add_devices(dev, 0, lm3533_cells, count, NULL, 0, NULL);
}

When this array is passed to mfd_add_devices(), the mfd core will attempt to
dereference a garbage pointer for the device name.  Can this cause memory
corruption or a kernel panic?


[Severity: Medium]
Can an invalid backlight reg value overlap with the LED reg address space?
Looking at lm3533_device_init(), the validation allows reg = 2 to be accepted
for a backlight:

drivers/mfd/lm3533-core.c:lm3533_device_init() {
        ...
        if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
            ret = fwnode_property_read_u32(child, "reg", &reg);
            if (ret || reg > LM3533_HVLED_ID_MAX) {
                dev_err(dev, "invalid backlight reg %d\n", reg);
                continue;
            }
        ...
}

LM3533_HVLED_ID_MAX is 2.  This incorrectly allows reg = 2 to be accepted for
a backlight, assigning it an ID of 2, even though the backlight driver only
supports 2 control banks (IDs 0 and 1).

While the backlight child driver safely returns -EINVAL during probe when it
encounters ID 2, should this check be reg >= LM3533_HVLED_ID_MAX to reject
invalid device tree configurations early in the core driver?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528135123.103745-1-clamor95@gmail.com?part=4