]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
5.4-stable patches
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 10 Mar 2023 13:05:28 +0000 (14:05 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 10 Mar 2023 13:05:28 +0000 (14:05 +0100)
added patches:
media-uvcvideo-fix-race-condition-with-usb_kill_urb.patch
media-uvcvideo-provide-sync-and-async-uvc_ctrl_status_event.patch

queue-5.4/media-uvcvideo-fix-race-condition-with-usb_kill_urb.patch [new file with mode: 0644]
queue-5.4/media-uvcvideo-provide-sync-and-async-uvc_ctrl_status_event.patch [new file with mode: 0644]
queue-5.4/series

diff --git a/queue-5.4/media-uvcvideo-fix-race-condition-with-usb_kill_urb.patch b/queue-5.4/media-uvcvideo-fix-race-condition-with-usb_kill_urb.patch
new file mode 100644 (file)
index 0000000..878b673
--- /dev/null
@@ -0,0 +1,140 @@
+From 619d9b710cf06f7a00a17120ca92333684ac45a8 Mon Sep 17 00:00:00 2001
+From: Ricardo Ribalda <ribalda@chromium.org>
+Date: Thu, 5 Jan 2023 15:31:29 +0100
+Subject: media: uvcvideo: Fix race condition with usb_kill_urb
+
+From: Ricardo Ribalda <ribalda@chromium.org>
+
+commit 619d9b710cf06f7a00a17120ca92333684ac45a8 upstream.
+
+usb_kill_urb warranties that all the handlers are finished when it
+returns, but does not protect against threads that might be handling
+asynchronously the urb.
+
+For UVC, the function uvc_ctrl_status_event_async() takes care of
+control changes asynchronously.
+
+If the code is executed in the following order:
+
+CPU 0                                  CPU 1
+=====                                  =====
+uvc_status_complete()
+                                       uvc_status_stop()
+uvc_ctrl_status_event_work()
+                                       uvc_status_start() -> FAIL
+
+Then uvc_status_start will keep failing and this error will be shown:
+
+<4>[    5.540139] URB 0000000000000000 submitted while active
+drivers/usb/core/urb.c:378 usb_submit_urb+0x4c3/0x528
+
+Let's improve the current situation, by not re-submiting the urb if
+we are stopping the status event. Also process the queued work
+(if any) during stop.
+
+CPU 0                                  CPU 1
+=====                                  =====
+uvc_status_complete()
+                                       uvc_status_stop()
+                                       uvc_status_start()
+uvc_ctrl_status_event_work() -> FAIL
+
+Hopefully, with the usb layer protection this should be enough to cover
+all the cases.
+
+Cc: stable@vger.kernel.org
+Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
+Reviewed-by: Yunke Cao <yunkec@chromium.org>
+Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
+Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/media/usb/uvc/uvc_ctrl.c   |    5 +++++
+ drivers/media/usb/uvc/uvc_status.c |   37 +++++++++++++++++++++++++++++++++++++
+ drivers/media/usb/uvc/uvcvideo.h   |    1 +
+ 3 files changed, 43 insertions(+)
+
+--- a/drivers/media/usb/uvc/uvc_ctrl.c
++++ b/drivers/media/usb/uvc/uvc_ctrl.c
+@@ -6,6 +6,7 @@
+  *          Laurent Pinchart (laurent.pinchart@ideasonboard.com)
+  */
++#include <asm/barrier.h>
+ #include <linux/kernel.h>
+ #include <linux/list.h>
+ #include <linux/module.h>
+@@ -1318,6 +1319,10 @@ static void uvc_ctrl_status_event_work(s
+       uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
++      /* The barrier is needed to synchronize with uvc_status_stop(). */
++      if (smp_load_acquire(&dev->flush_status))
++              return;
++
+       /* Resubmit the URB. */
+       w->urb->interval = dev->int_ep->desc.bInterval;
+       ret = usb_submit_urb(w->urb, GFP_KERNEL);
+--- a/drivers/media/usb/uvc/uvc_status.c
++++ b/drivers/media/usb/uvc/uvc_status.c
+@@ -6,6 +6,7 @@
+  *          Laurent Pinchart (laurent.pinchart@ideasonboard.com)
+  */
++#include <asm/barrier.h>
+ #include <linux/kernel.h>
+ #include <linux/input.h>
+ #include <linux/slab.h>
+@@ -310,5 +311,41 @@ int uvc_status_start(struct uvc_device *
+ void uvc_status_stop(struct uvc_device *dev)
+ {
++      struct uvc_ctrl_work *w = &dev->async_ctrl;
++
++      /*
++       * Prevent the asynchronous control handler from requeing the URB. The
++       * barrier is needed so the flush_status change is visible to other
++       * CPUs running the asynchronous handler before usb_kill_urb() is
++       * called below.
++       */
++      smp_store_release(&dev->flush_status, true);
++
++      /*
++       * Cancel any pending asynchronous work. If any status event was queued,
++       * process it synchronously.
++       */
++      if (cancel_work_sync(&w->work))
++              uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
++
++      /* Kill the urb. */
+       usb_kill_urb(dev->int_urb);
++
++      /*
++       * The URB completion handler may have queued asynchronous work. This
++       * won't resubmit the URB as flush_status is set, but it needs to be
++       * cancelled before returning or it could then race with a future
++       * uvc_status_start() call.
++       */
++      if (cancel_work_sync(&w->work))
++              uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
++
++      /*
++       * From this point, there are no events on the queue and the status URB
++       * is dead. No events will be queued until uvc_status_start() is called.
++       * The barrier is needed to make sure that flush_status is visible to
++       * uvc_ctrl_status_event_work() when uvc_status_start() will be called
++       * again.
++       */
++      smp_store_release(&dev->flush_status, false);
+ }
+--- a/drivers/media/usb/uvc/uvcvideo.h
++++ b/drivers/media/usb/uvc/uvcvideo.h
+@@ -664,6 +664,7 @@ struct uvc_device {
+       /* Status Interrupt Endpoint */
+       struct usb_host_endpoint *int_ep;
+       struct urb *int_urb;
++      bool flush_status;
+       u8 *status;
+       struct input_dev *input;
+       char input_phys[64];
diff --git a/queue-5.4/media-uvcvideo-provide-sync-and-async-uvc_ctrl_status_event.patch b/queue-5.4/media-uvcvideo-provide-sync-and-async-uvc_ctrl_status_event.patch
new file mode 100644 (file)
index 0000000..ecdd9e7
--- /dev/null
@@ -0,0 +1,107 @@
+From d9c8763e61295be0a21dc04ad9c379d5d17c3d86 Mon Sep 17 00:00:00 2001
+From: Ricardo Ribalda <ribalda@chromium.org>
+Date: Wed, 23 Dec 2020 14:35:20 +0100
+Subject: media: uvcvideo: Provide sync and async uvc_ctrl_status_event
+
+From: Ricardo Ribalda <ribalda@chromium.org>
+
+commit d9c8763e61295be0a21dc04ad9c379d5d17c3d86 upstream.
+
+Split the functionality of void uvc_ctrl_status_event_work in two, so it
+can be called by functions outside interrupt context and not part of an
+URB.
+
+Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
+Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/media/usb/uvc/uvc_ctrl.c   |   25 +++++++++++++++----------
+ drivers/media/usb/uvc/uvc_status.c |    3 ++-
+ drivers/media/usb/uvc/uvcvideo.h   |    4 +++-
+ 3 files changed, 20 insertions(+), 12 deletions(-)
+
+--- a/drivers/media/usb/uvc/uvc_ctrl.c
++++ b/drivers/media/usb/uvc/uvc_ctrl.c
+@@ -1275,17 +1275,12 @@ static void uvc_ctrl_send_slave_event(st
+       uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
+ }
+-static void uvc_ctrl_status_event_work(struct work_struct *work)
++void uvc_ctrl_status_event(struct uvc_video_chain *chain,
++                         struct uvc_control *ctrl, const u8 *data)
+ {
+-      struct uvc_device *dev = container_of(work, struct uvc_device,
+-                                            async_ctrl.work);
+-      struct uvc_ctrl_work *w = &dev->async_ctrl;
+-      struct uvc_video_chain *chain = w->chain;
+       struct uvc_control_mapping *mapping;
+-      struct uvc_control *ctrl = w->ctrl;
+       struct uvc_fh *handle;
+       unsigned int i;
+-      int ret;
+       mutex_lock(&chain->ctrl_mutex);
+@@ -1293,7 +1288,7 @@ static void uvc_ctrl_status_event_work(s
+       ctrl->handle = NULL;
+       list_for_each_entry(mapping, &ctrl->info.mappings, list) {
+-              s32 value = __uvc_ctrl_get_value(mapping, w->data);
++              s32 value = __uvc_ctrl_get_value(mapping, data);
+               /*
+                * handle may be NULL here if the device sends auto-update
+@@ -1312,6 +1307,16 @@ static void uvc_ctrl_status_event_work(s
+       }
+       mutex_unlock(&chain->ctrl_mutex);
++}
++
++static void uvc_ctrl_status_event_work(struct work_struct *work)
++{
++      struct uvc_device *dev = container_of(work, struct uvc_device,
++                                            async_ctrl.work);
++      struct uvc_ctrl_work *w = &dev->async_ctrl;
++      int ret;
++
++      uvc_ctrl_status_event(w->chain, w->ctrl, w->data);
+       /* Resubmit the URB. */
+       w->urb->interval = dev->int_ep->desc.bInterval;
+@@ -1321,8 +1326,8 @@ static void uvc_ctrl_status_event_work(s
+                          ret);
+ }
+-bool uvc_ctrl_status_event(struct urb *urb, struct uvc_video_chain *chain,
+-                         struct uvc_control *ctrl, const u8 *data)
++bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain,
++                               struct uvc_control *ctrl, const u8 *data)
+ {
+       struct uvc_device *dev = chain->dev;
+       struct uvc_ctrl_work *w = &dev->async_ctrl;
+--- a/drivers/media/usb/uvc/uvc_status.c
++++ b/drivers/media/usb/uvc/uvc_status.c
+@@ -179,7 +179,8 @@ static bool uvc_event_control(struct urb
+       switch (status->bAttribute) {
+       case UVC_CTRL_VALUE_CHANGE:
+-              return uvc_ctrl_status_event(urb, chain, ctrl, status->bValue);
++              return uvc_ctrl_status_event_async(urb, chain, ctrl,
++                                                 status->bValue);
+       case UVC_CTRL_INFO_CHANGE:
+       case UVC_CTRL_FAILURE_CHANGE:
+--- a/drivers/media/usb/uvc/uvcvideo.h
++++ b/drivers/media/usb/uvc/uvcvideo.h
+@@ -833,7 +833,9 @@ int uvc_ctrl_add_mapping(struct uvc_vide
+ int uvc_ctrl_init_device(struct uvc_device *dev);
+ void uvc_ctrl_cleanup_device(struct uvc_device *dev);
+ int uvc_ctrl_restore_values(struct uvc_device *dev);
+-bool uvc_ctrl_status_event(struct urb *urb, struct uvc_video_chain *chain,
++bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain,
++                               struct uvc_control *ctrl, const u8 *data);
++void uvc_ctrl_status_event(struct uvc_video_chain *chain,
+                          struct uvc_control *ctrl, const u8 *data);
+ int uvc_ctrl_begin(struct uvc_video_chain *chain);
index ef373a0b8a053614bc6267a0761932be4e0cfac8..0df41717bcf6ef1b8d0d2e7790f525de113b6590 100644 (file)
@@ -351,3 +351,5 @@ x86-resctrl-apply-read_once-write_once-to-task_struct.-rmid-closid.patch
 x86-resctl-fix-scheduler-confusion-with-current.patch
 bluetooth-hci_sock-purge-socket-queues-in-the-destruct-callback.patch
 tcp-fix-listen-regression-in-5.15.88.patch
+media-uvcvideo-provide-sync-and-async-uvc_ctrl_status_event.patch
+media-uvcvideo-fix-race-condition-with-usb_kill_urb.patch