Re: [PATCH v11 2/3] hwmon: ltc4283: Add support for the LTC4283 Swap Controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Use-After-Free due to lingering debugfs files after driver unbind or probe failure
- [High] Race condition causing lost hardware alarms via Read-Modify-Write (RMW) on status registers
- [High] NULL pointer dereference in the auxiliary GPIO driver due to missing platform data
- [Low] Missing documentation for the `in0_reset_history` sysfs attribute
--

commit fe2f05d89e51b9f3d68748be92daeb1de9889838
Author: Nuno Sá <nuno.sa@analog.com>
hwmon: ltc4283: Add support for the LTC4283 Swap Controller

This commit adds support for the LTC4283 Hot Swap Controller, providing
programmable current limit, SOA timer, and I2C/ADC monitoring capabilities.

> diff --git a/Documentation/hwmon/ltc4283.rst b/Documentation/hwmon/ltc4283.rst
> --- /dev/null
> +++ b/Documentation/hwmon/ltc4283.rst
> @@ -39,6 +39,10 @@
>  ======================= ==========================================
>  in0_lcrit_alarm         Critical Undervoltage alarm
>  in0_crit_alarm          Critical Overvoltage alarm
>  in0_label		Channel label (VIN)
>  
>  in1_input		Output voltage (mV).

This isn't a bug, but should in0_reset_history be documented here? The
channel info array exposes HWMON_I_RESET_HISTORY for channel 0, and the
driver implements it, but it appears to be missing from the documentation.

> diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c
> --- /dev/null
> +++ b/drivers/hwmon/ltc4283.c

[ ... ]

> +static int ltc4283_read_alarm(struct ltc4283_hwmon *st, u32 reg,
> +			      u32 mask, long *val)
> +{
> +	u32 alarm;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &alarm);
> +	if (ret)
> +		return ret;
> +
> +	*val = !!(alarm & mask);
> +
> +	/* If not status/fault logs, clear the alarm after reading it. */
> +	if (reg != LTC4283_FAULT_STATUS && reg != LTC4283_FAULT_LOG)
> +		return regmap_write(st->map, reg, alarm & ~mask);
> +
> +	return 0;
> +}

Can this sequence cause other alarms to be lost if they are asserted by the
hardware between the read and the write? 

Writing back the modified alarm value could inadvertently clear new alarms
that were just set. Would it be safer to write a constant mask directly
without a read-modify-write cycle?

[ ... ]

> +static int ltc4283_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev, *hwmon;
> +	struct auxiliary_device *adev;
> +	struct ltc4283_hwmon *st;
> +	int ret, id;
> +
> +	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;

[ ... ]

> +	ltc4283_debugfs_init(st, client);

Could this lead to a Use-After-Free if the driver is manually unbound? 

The private data st is allocated with devm_kzalloc, so it is freed when the
driver unbinds. However, ltc4283_debugfs_init attaches debugfs files to
the I2C client's debugfs directory, which persists until the device itself
is removed.

If a user space read occurs on those files after the driver unbinds, could
it dereference the freed pointer?

> +
> +	if (!st->gpio_mask)
> +		return 0;
> +
> +	id = (client->adapter->nr << 10) | client->addr;
> +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
> +					      NULL, id);

Will this cause a NULL pointer dereference in the auxiliary GPIO driver? 

The auxiliary device is created with NULL passed for the platform data.
If the GPIO driver expects valid platform data to initialize its valid mask
via dev_get_platdata, passing NULL here could result in a crash during
bitmap copying.

> +	if (!adev)
> +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO device\n");
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429-ltc4283-support-v11-0-27ccde619dad@analog.com?part=2