* [PATCH 0/3] USB: usb-skeleton: regression fix @ 2019-10-09 17:09 Johan Hovold 2019-10-09 17:09 ` [PATCH 1/3] USB: usb-skeleton: fix NULL-deref on disconnect Johan Hovold ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Johan Hovold @ 2019-10-09 17:09 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold 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(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] USB: usb-skeleton: fix NULL-deref on disconnect 2019-10-09 17:09 [PATCH 0/3] USB: usb-skeleton: regression fix Johan Hovold @ 2019-10-09 17:09 ` Johan Hovold 2019-10-09 17:09 ` [PATCH 2/3] USB: usb-skeleton: fix use-after-free after driver unbind Johan Hovold 2019-10-09 17:09 ` [PATCH 3/3] USB: usb-skeleton: drop redundant in-urb check Johan Hovold 2 siblings, 0 replies; 4+ messages in thread From: Johan Hovold @ 2019-10-09 17:09 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold, stable 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. Fixes: 4212cd74ca6f ("USB: usb-skeleton.c: remove err() usage") Fixes: 5c290a5e42c3 ("USB: usb-skeleton: fix runtime PM after driver unbind") Cc: stable <stable@vger.kernel.org> Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/usb-skeleton.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index 8001d6384c73..c2843fcfa52d 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -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); -- 2.23.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] USB: usb-skeleton: fix use-after-free after driver unbind 2019-10-09 17:09 [PATCH 0/3] USB: usb-skeleton: regression fix Johan Hovold 2019-10-09 17:09 ` [PATCH 1/3] USB: usb-skeleton: fix NULL-deref on disconnect Johan Hovold @ 2019-10-09 17:09 ` Johan Hovold 2019-10-09 17:09 ` [PATCH 3/3] USB: usb-skeleton: drop redundant in-urb check Johan Hovold 2 siblings, 0 replies; 4+ messages in thread From: Johan Hovold @ 2019-10-09 17:09 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold The 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. Fixes: e7389cc9a7ff ("USB: skel_read really sucks royally") Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/usb-skeleton.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index c2843fcfa52d..be311787403e 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -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 */ -- 2.23.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] USB: usb-skeleton: drop redundant in-urb check 2019-10-09 17:09 [PATCH 0/3] USB: usb-skeleton: regression fix Johan Hovold 2019-10-09 17:09 ` [PATCH 1/3] USB: usb-skeleton: fix NULL-deref on disconnect Johan Hovold 2019-10-09 17:09 ` [PATCH 2/3] USB: usb-skeleton: fix use-after-free after driver unbind Johan Hovold @ 2019-10-09 17:09 ` Johan Hovold 2 siblings, 0 replies; 4+ messages in thread From: Johan Hovold @ 2019-10-09 17:09 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold The 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(). Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/usb/usb-skeleton.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c index be311787403e..2dc58766273a 100644 --- a/drivers/usb/usb-skeleton.c +++ b/drivers/usb/usb-skeleton.c @@ -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 */ -- 2.23.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-09 17:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-09 17:09 [PATCH 0/3] USB: usb-skeleton: regression fix Johan Hovold 2019-10-09 17:09 ` [PATCH 1/3] USB: usb-skeleton: fix NULL-deref on disconnect Johan Hovold 2019-10-09 17:09 ` [PATCH 2/3] USB: usb-skeleton: fix use-after-free after driver unbind Johan Hovold 2019-10-09 17:09 ` [PATCH 3/3] USB: usb-skeleton: drop redundant in-urb check Johan Hovold
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.