Re: issue with devm_ methods in disconnect and freeing memory in btusb

[Oliver Neukum <oliver@neukum.org>]

On 14.11.25 19:11, Raphael Pinsonneault-Thibeault wrote:
> On Fri, Nov 14, 2025 at 09:03:45AM +0100, Oliver Neukum wrote:


>> We must always be ready to handle any arbitrary order.
>> The code in the second branch of the if statement used to be perfectly correct.
>> The actual breaking commit was 98921dbd00c4e by introducing devm_kzalloc()
>> for memory allocation. That ties the lifetime of memory to the binding
>> of a driver to an interface. In hindsight in a driver that binds
>> to multiple interfaces, this is problematic. Hence I would propose
>> to just revert 98921dbd00c4e. It seems to me that we have discovered
>> a design limitation in the devm_ methods. What do you think?
>>
>> 	Regards
>> 		Oliver
>>
> 
> Hi Oliver,
> 
> I was under the impression that tying the memory lifetime to the INTF
> interface was intentional since 98921dbd00c4e was from 2012, hence my

No, it was not. And it would be kind of defeating the purpose
of a devm-method. You use them to not have to free memory manually.
In the disconnect() method of btusb we do free memory manually,
only that we do so implicitly. And that is dangerous.

> commit message in 23d22f2f7176:
> 
> -- quote --
> 
> There is a KASAN: slab-use-after-free read in btusb_disconnect().
> Calling "usb_driver_release_interface(&btusb_driver, data->intf)" will

Was that obvious to you?

> free the btusb data associated with the interface. The same data is
> then used later in the function, hence the UAF.
> 
> Fix by moving the accesses to btusb data to before the data is free'd.
> 
> -- quote --
> 
> However, it seems that support for other interfaces was added
> later, e.g. by 9d08f50401ac7, and out-of-band wakeup support even later
> by fd913ef7ce619.

ISO support is very old. It is necessary because you cannot
change the altsetting of an interface that is in use. It is
a design flaw of USB.
> So, maybe it just wasn't considered?
Indeed and hence I would just revert it.
This is an accident waiting to happen.

	Regards
		Oliver