2024-06-14 12:55:03

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH v6 0/4] uvcvideo: Attempt N to land UVC race conditions fixes

Back in 2020 Guenter published a set of patches to fix some race
conditions in UVC:
https://lore.kernel.org/all/[email protected]/

That kind of race conditions are not only seen in UVC, but are a common
seen in almost all the kernel, so this is what it was decided back then
that we should try to fix them at higher levels.

After that. A lot of video_is_registered() were added to the core:

```
ribalda@alco:~/work/linux$ git grep is_registered drivers/media/v4l2-core/
drivers/media/v4l2-core/v4l2-compat-ioctl32.c: if (!video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev)) {
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (!video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (vdev == NULL || !video_is_registered(vdev)) {
drivers/media/v4l2-core/v4l2-dev.c: if (video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-dev.c: if (!vdev || !video_is_registered(vdev))
drivers/media/v4l2-core/v4l2-ioctl.c: if (!video_is_registered(vfd)) {
drivers/media/v4l2-core/v4l2-subdev.c: if (video_is_registered(vdev)) {
```

And recently Sakari is trying to land:
https://lore.kernel.org/linux-media/[email protected]/

Which will make obsolete a lot off (all?) of the video_is_registered() checks in
Guenter's patches.

Besides those checks, there were some other valid races fixed in his
patches.

This patchset tries to fix the races still present in our code.

Thanks!

Signed-off-by: Ricardo Ribalda <[email protected]>
---
Changes in v6: Thanks Hans
- s/uvc_queue_streamoff/uvc_queue_release/
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5: Thanks Hans!
- Refactor unregister as vb2_video_unregister_device
- I have tested the first patch independently from the others, so it
could be merged in two steps if needed.
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4: Thanks Sergey and Guenter
- Fix typos
- Move location of mutex_init
- Split patch to make the suspend change explicit
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3: Thanks Hans!
- Stop streaming during uvc_unregister()
- Refactor the uvc_status code
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Actually send the series to the ML an not only to individuals.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Ricardo Ribalda (4):
media: uvcvideo: Stop stream during unregister
media: uvcvideo: Refactor the status irq API
media: uvcvideo: Avoid race condition during unregister
media: uvcvideo: Exit early if there is not int_urb

drivers/media/usb/uvc/uvc_driver.c | 45 +++++++++++++++++++--------
drivers/media/usb/uvc/uvc_status.c | 62 +++++++++++++++++++++++++++++++++++---
drivers/media/usb/uvc/uvc_v4l2.c | 22 ++++----------
drivers/media/usb/uvc/uvcvideo.h | 10 +++---
4 files changed, 103 insertions(+), 36 deletions(-)
---
base-commit: b14257abe7057def6127f6fb2f14f9adc8acabdb
change-id: 20230309-guenter-mini-89861b084ef1

Best regards,
--
Ricardo Ribalda <[email protected]>