Re: [PATCH net-next v3 2/2] net: pse-pd: Add LTC4266 PSE controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pse: incorrect devres registration order causes IRQ use-after-free
- [High] pse: swallowing -EPROBE_DEFER causes incorrect I_CUT limit calculation
--

commit 1aad2f6b5a72ee34250a04fe6ad2b8aea94fa801
Author: Kyle Swenson <kyle.swenson@est.tech>

net: pse-pd: Add LTC4266 PSE controller driver

Add a new driver for the Linear Technology LTC4266 I2C Power Sourcing
Equipment controller and integrate with the PSE controller core. The
LTC4266 supports Type 1 and Type 2 Powered Devices.

> diff --git a/drivers/net/pse-pd/ltc4266.c b/drivers/net/pse-pd/ltc4266.c
> index 0000000000000..37dea467811a4
> --- /dev/null
> +++ b/drivers/net/pse-pd/ltc4266.c
[ ... ]
> +static int ltc4266_pi_nominal_uv(struct ltc4266 *ltc4266, struct device_node *np)
> +{
> +	struct regulator *vpwr;
> +	int uv;
> +
> +	vpwr = of_regulator_get_optional(ltc4266->dev, np, "vpwr");
> +	if (IS_ERR(vpwr)) {
> +		/* -ENODEV means the PI describes no vpwr-supply at all, which
> +		 * is the case the fallback exists for. -EPROBE_DEFER means the
> +		 * rail _is_ described but has not registered yet, so the
> +		 * fallback is wrong for it. Asking for a probe retry is not an
> +		 * option from here: we run from setup_pi_matrix(), and
> +		 * pse_controller_register() unwinds neither its notification
> +		 * fifo nor its pse_pi array when that fails, so every retry
> +		 * would leak. Warn instead so the assumed voltage is visible.
> +		 */
> +		if (PTR_ERR(vpwr) == -EPROBE_DEFER)
> +			dev_warn(ltc4266->dev,
> +				 "%pOF: vpwr-supply is not registered yet, assuming %d uV\n",
> +				 np, LTC4266_VPORT_NOMINAL_UV);
> +
> +		return LTC4266_VPORT_NOMINAL_UV;

[Severity: High]
Does swallowing -EPROBE_DEFER here cause an incorrect I_CUT limit calculation?

If the vpwr-supply regulator is not yet registered when the LTC4266 driver
probes, of_regulator_get_optional() returns -EPROBE_DEFER. By suppressing
this error and falling back to a hardcoded 50V nominal voltage, the port
might operate with an incorrect, permanently-cached power limit. This could
result in delivering more power than administratively allowed, potentially
damaging equipment or violating PoE budgets.

[ ... ]
> +static int ltc4266_probe(struct i2c_client *client)
> +{
[ ... ]
> +	ret = devm_pse_controller_register(ltc4266->dev, &ltc4266->pcdev);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to register PSE controller\n");
> +
> +	/* Enable the interrupt pin, and only report detect events on
> +	 * change (detchg) so idle ports continuously re-running
> +	 * detection in semi-auto mode don't flood the host with a
> +	 * detect event every cycle.
> +	 */
> +	ret = regmap_update_bits(ltc4266->regmap, LTC4266_REG_MCONF,
> +				 LTC4266_MCONF_INTERRUPT_ENABLE | LTC4266_MCONF_DETCHG,
> +				 LTC4266_MCONF_INTERRUPT_ENABLE | LTC4266_MCONF_DETCHG);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to configure interrupts\n");
> +
> +	ret = devm_pse_irq_helper(&ltc4266->pcdev, client->irq,
> +				  0, &irq_desc);

[Severity: High]
Will this devres registration order cause an IRQ use-after-free during unbind?

Because devm_pse_controller_register() is called before devm_pse_irq_helper()
in ltc4266_probe(), the devres LIFO ordering will free the IRQ first during
module removal or probe failure teardown.

After the IRQ is freed, the devres teardown will unregister the controller,
which unconditionally calls disable_irq(pcdev->irq) inside of
pse_controller_unregister(). If the IRQ was reassigned to another device in
the meantime, this could disable that device's interrupt and cause a denial
of service.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927212929.593366-1-kyle.swenson@est.tech?part=2