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

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

[Guenter Roeck]

On 4/30/26 03:03, sashiko-bot@kernel.org wrote:
...
>> +	id = (client->adapter->nr << 10) | client->addr;
>> +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
>> +					      &st->gpio_mask, id);
>> +	if (!adev)
>> +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO device\n");
> 
> Can this error path lead to a use-after-free?
> 
> According to the I2C subsystem rules, debugfs entries attached to the
> client debugfs directory are only cleaned up during the client device
> removal function on driver unbind.
> 
> If __devm_auxiliary_device_create() fails, the probe function aborts and
> the devres subsystem frees the st pointer. However, the debugfs files
> persist and retain operations that operate on the freed st pointer.
> 

AI taking everything literally :-(. I'll have to reword that guidance again
to include the probe error path.

Guenter

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

[Nuno Sá]

On Thu, Apr 30, 2026 at 07:52:55AM -0700, Guenter Roeck wrote:
> On 4/30/26 03:03, sashiko-bot@kernel.org wrote:
> ...
> > > +	id = (client->adapter->nr << 10) | client->addr;
> > > +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
> > > +					      &st->gpio_mask, id);
> > > +	if (!adev)
> > > +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO device\n");
> > 
> > Can this error path lead to a use-after-free?
> > 
> > According to the I2C subsystem rules, debugfs entries attached to the
> > client debugfs directory are only cleaned up during the client device
> > removal function on driver unbind.
> > 
> > If __devm_auxiliary_device_create() fails, the probe function aborts and
> > the devres subsystem frees the st pointer. However, the debugfs files
> > persist and retain operations that operate on the freed st pointer.
> > 
> 
> AI taking everything literally :-(. I'll have to reword that guidance again
> to include the probe error path.
> 

Yeah and it is annoying it always comes up with something that could
have been said before. Even more annoying some of are legit issues :)

- Nuno Sá

> Guenter
> 

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

[Guenter Roeck]

On 4/30/26 09:50, Nuno Sá wrote:
> On Thu, Apr 30, 2026 at 07:52:55AM -0700, Guenter Roeck wrote:
>> On 4/30/26 03:03, sashiko-bot@kernel.org wrote:
>> ...
>>>> +	id = (client->adapter->nr << 10) | client->addr;
>>>> +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
>>>> +					      &st->gpio_mask, id);
>>>> +	if (!adev)
>>>> +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO device\n");
>>>
>>> Can this error path lead to a use-after-free?
>>>
>>> According to the I2C subsystem rules, debugfs entries attached to the
>>> client debugfs directory are only cleaned up during the client device
>>> removal function on driver unbind.
>>>
>>> If __devm_auxiliary_device_create() fails, the probe function aborts and
>>> the devres subsystem frees the st pointer. However, the debugfs files
>>> persist and retain operations that operate on the freed st pointer.
>>>
>>
>> AI taking everything literally :-(. I'll have to reword that guidance again
>> to include the probe error path.
>>
> 
> Yeah and it is annoying it always comes up with something that could
> have been said before. Even more annoying some of are legit issues :)
> 

Agreed, but I think it it is better that it finds the issues in multiple
rounds than not finding them at all. And I have to say it is very human -
the same happens to me as well when I do code reviews of complex patches.
A major difference is that, unlike me, AI doesn't get tired.

Guenter

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

[Nuno Sá]

On Thu, 2026-04-30 at 23:10 -0700, Guenter Roeck wrote:
> On 4/30/26 09:50, Nuno Sá wrote:
> > On Thu, Apr 30, 2026 at 07:52:55AM -0700, Guenter Roeck wrote:
> > > On 4/30/26 03:03, sashiko-bot@kernel.org wrote:
> > > ...
> > > > > +	id = (client->adapter->nr << 10) | client->addr;
> > > > > +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
> > > > > +					      &st->gpio_mask, id);
> > > > > +	if (!adev)
> > > > > +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO
> > > > > device\n");
> > > > 
> > > > Can this error path lead to a use-after-free?
> > > > 
> > > > According to the I2C subsystem rules, debugfs entries attached to the
> > > > client debugfs directory are only cleaned up during the client device
> > > > removal function on driver unbind.
> > > > 
> > > > If __devm_auxiliary_device_create() fails, the probe function aborts and
> > > > the devres subsystem frees the st pointer. However, the debugfs files
> > > > persist and retain operations that operate on the freed st pointer.
> > > > 
> > > 
> > > AI taking everything literally :-(. I'll have to reword that guidance again
> > > to include the probe error path.
> > > 
> > 
> > Yeah and it is annoying it always comes up with something that could
> > have been said before. Even more annoying some of are legit issues :)
> > 
> 
> Agreed, but I think it it is better that it finds the issues in multiple
> rounds than not finding them at all. And I have to say it is very human -
> the same happens to me as well when I do code reviews of complex patches.
> A major difference is that, unlike me, AI doesn't get tired.
> 

Yes, nothing I can disagree with. Until it continues to come up with legit reports, I
just need to continue to re-spin :)

- Nuno Sá