On 06.12.21 21:57, Philipp Hortmann wrote:
> Update comment: decrement our usage count ..
> and code according to usb-skeleton.c
Hi,
and that is exactly the problem, I am afraid.
Your patch would be correct if the underlying code were correct.
>
> - /* decrement our usage count for the device */
> - --skel->open_count;
> + /* decrement the count on our device */
> + kref_put(&dev->kref, skel_delete);
>
>
> One of the more difficult problems that USB drivers must be able to
I am sorry but the code in usb-skel.c is wrong. You grab a reference
in skel_open():
/* increment our usage count for the device */
kref_get(&dev->kref);
which is good, but in skel_release() we do:
/* decrement the count on our device */
kref_put(&dev->kref, skel_delete);
unconditionally.
Think this through:
- Device is plugged in -> device node and internal data is created
- open() called -> kref_get(), we get a reference
- close() -> kref_put() -> refcount goes to zero -> skel_delete() is called, struct usb_skel is freed:
static void skel_delete(struct kref *kref)
{
struct usb_skel *dev = to_skel_dev(kref);
usb_free_urb(dev->bulk_in_urb);
usb_put_intf(dev->interface);
usb_put_dev(dev->udev);
kfree(dev->bulk_in_buffer);
kfree(dev);
}
with intfdata left intact.
- open() is called again -> We are following a dangling pointer into cloud cuckoo land.
Unfortunately this code is older than git, so I cannot just send a revert.
What to do?
Regards
Oliver
