USB: usb-skeleton: regression fix
Subsystems (2)
cover USB: usb-skeleton: regression fix
I messed up when submitting the runtime PM fixes last week and failed to notice that the change to usb-skeleton depended on another fix I already had in my tree (I did notice the conflict, but rebased and sent a v2 also without the prerequisite patch).
So here's a regression fix to a commit in usb-linus for usb-skeleton. :/
Included are also a use-after-free fix and a related clean up.
Johan
Johan Hovold (3):
USB: usb-skeleton: fix NULL-deref on disconnect
USB: usb-skeleton: fix use-after-free after driver unbind
USB: usb-skeleton: drop redundant in-urb check
drivers/usb/usb-skeleton.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Signature · 1 line
2.23.0
PATCH 1/3 USB: usb-skeleton: fix NULL-deref on disconnect
The driver was using its struct usb_interface pointer as an inverted disconnected flag and was setting it to NULL before making sure all completion handlers had run. This could lead to NULL-pointer dereferences in the dev_err() statements in the completion handlers which relies on said pointer.
Fix this by using a dedicated disconnected flag.
Note that this is also addresses a NULL-pointer dereference at release() and a struct usb_interface reference leak introduced by a recent runtime PM fix, which depends on and should have been submitted together with this patch.
--- drivers/usb/usb-skeleton.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/usb/usb-skeleton.c +4 -3
@@ -61,6 +61,7 @@ struct usb_skel {spinlock_t err_lock;/* lock for errors */ struct kref kref;struct mutex io_mutex;/* synchronize I/O with disconnect */ + unsigned long disconnected:1;wait_queue_head_t bulk_in_wait;/* to wait for an ongoing read */ };#define to_skel_dev (d) container_of(d, struct usb_skel, kref)
@@ -238,7 +239,7 @@ static ssize_t skel_read(struct file *file, char *buffer, size_t count,if (rv <0 )return rv; -if (!dev->interface) {/* disconnect() was called */ +if (dev->disconnected) {/* disconnect() was called */ rv = -ENODEV ; goto exit; }
@@ -420,7 +421,7 @@ static ssize_t skel_write(struct file *file, const char *user_buffer,/* this lock makes sure we don't submit URBs to gone devices */ mutex_lock (&dev->io_mutex); -if (!dev->interface) {/* disconnect() was called */ +if (dev->disconnected) {/* disconnect() was called */ mutex_unlock (&dev->io_mutex); retval = -ENODEV ; goto error;
@@ -571,7 +572,7 @@ static void skel_disconnect(struct usb_interface *interface)/* prevent more I/O from starting */ mutex_lock (&dev->io_mutex); - dev->interface =NULL ; + dev->disconnected =1 ;mutex_unlock (&dev->io_mutex);usb_kill_anchored_urbs (&dev->submitted);
Signature · 1 line
2.23.0
PATCH 2/3 USB: usb-skeleton: fix use-after-free after driver unbind
In reply to Johan HovoldThe driver failed to stop its read URB on disconnect, something which could lead to a use-after-free in the completion handler after driver unbind in case the character device has been closed.
--- drivers/usb/usb-skeleton.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/usb-skeleton.c +1 -0
@@ -575,6 +575,7 @@ static void skel_disconnect(struct usb_interface *interface) dev->disconnected =1 ;mutex_unlock (&dev->io_mutex); +usb_kill_urb (dev->bulk_in_urb);usb_kill_anchored_urbs (&dev->submitted);/* decrement our usage count */
Signature · 1 line
2.23.0
PATCH 3/3 USB: usb-skeleton: drop redundant in-urb check
In reply to Johan HovoldThe driver bails out at probe if we can't find a bulk-in endpoint or if we fail to allocate the URB, so drop the check in read().
--- drivers/usb/usb-skeleton.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/usb/usb-skeleton.c +1 -2
@@ -230,8 +230,7 @@ static ssize_t skel_read(struct file *file, char *buffer, size_t count, dev = file->private_data; -/* if we cannot read at all, return EOF */ -if (!dev->bulk_in_urb || !count) +if (!count)return 0 ;/* no concurrent readers */
Signature · 1 line
2.23.0