Messages in this thread | | | Subject | Re: [PATCH v3 2/5] Docs: usb: update comment and code near decrement our usage count for the device | From | Oliver Neukum <> | Date | Tue, 7 Dec 2021 10:30:26 +0100 |
| |
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
| |