issue with devm_ methods in disconnect and freeing memory in btusb

[Oliver Neukum <oliver@neukum.org>]

Hi,

looking at the change to btusb_disconnect() in 23d22f2f7176
and the discussion leading to it I have doubts. Let me quote the change log:

-- quote --

Syzbot opens a usb device with out of order interface descriptors:
Interface 3 (ISOC) in position 0, Interface 2 (DIAG) in position 1,
Interface 1 (INTF) in position 2.
So, ISOC is the first interface to get disconnected by usb_disconnect()
-> usb_disable_device() -> ... -> btusb_disconnect().

I don't think this is a problem on hardware, where the bInterfaceNumber
matches the position in the dev->actconfig->interface list; and in
btusb_disconnect() it would only ever go into the first if
statement: "if (intf == data->intf)" and not into any of the others.

-- quote --

Now, we cannot do this. The order disconnect() is called is arbitrary

1. The order syzbot used is valid according to spec, albeit unusual
2. disconnect() can be triggered from user space via sysfs

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