September 2026 - media-ci

Re: [v2,0/3] media: Two oopses and a hang when unbinding a streaming sensor
by Patchwork Integration 11 Sep '26

11 Sep '26

Dear Nicola Fiorillo: Thanks for your patches! Unfortunately the Media CI robot detected some issues: # Test virtme64:test-virtme Error running test. Final Summary not found Please fix your series, and upload a new version. If you have a patchwork account, do not forget to mark the current series as Superseded. For more details, check the full report at: https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/11010994… . Best regards, and Happy Hacking! Media CI robot on behalf of the linux-media community. --- Check the latest rules for contributing your patches at: https://docs.kernel.org/driver-api/media/maintainer-entry-profile.html If you believe that the CI is wrong, kindly open an issue at https://gitlab.freedesktop.org/linux-media/media-ci/-/issues or reply-all to this message.

1 0

Re: [PATCH v2 1/3] media: ipu6: Check the remote pad before dereferencing it
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Unconditional dereference of `media_pad_remote_pad_first` result in IPU7 CSI2 driver. - [High] Lockless traversal of media graph links and use of `remote_sd` causes a Use-After-Free during concurrent sensor unbind. -- commit 240e809b82fcb6a1a3d2aa43372646cb17e7ee9c Author: Nicola Fiorillo <nicfio(a)gmail.com> media: ipu6: Check the remote pad before dereferencing it This commit fixes a NULL pointer dereference in the IPU6 CSI-2 driver that occurs when a sensor is unbound during capture. It checks the return value of media_pad_remote_pad_first() in the stream enable and disable paths, refusing to start if there is no link and stopping the receiver without notifying the missing sensor. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > index 7e539a0c6c92f..c00a82eb8d4e5 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > @@ -356,6 +356,9 @@ static int ipu6_isys_csi2_enable_streams(struct v4l2_subdev *sd, > int ret; > > remote_pad = media_pad_remote_pad_first(&sd->entity.pads[CSI2_PAD_SINK]); > + if (!remote_pad) > + return -ENOLINK; > + > remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity); [Severity: High] This isn't a bug introduced by this patch, but does this lockless access create a use-after-free race during concurrent sensor unbind? In ipu6_isys_csi2_enable_streams(), media_pad_remote_pad_first() iterates over entity->links without holding mdev->graph_mutex. Even if the iterator succeeds, the pointer remote_sd is obtained locklessly. Can the unbind thread free the sensor's private struct (and embedded v4l2_subdev) before the stream function passes it to v4l2_subdev_enable_streams()? [ ... ] > @@ -392,10 +395,17 @@ static int ipu6_isys_csi2_disable_streams(struct v4l2_subdev *sd, > v4l2_subdev_state_xlate_streams(state, pad, CSI2_PAD_SINK, > &streams_mask); > > + ipu6_isys_csi2_set_stream(sd, NULL, 0, false); > + > + /* > + * The link is gone if the sensor driver was unbound while streaming. > + * Stop the receiver anyway, there is just no one left to tell. > + */ > remote_pad = media_pad_remote_pad_first(&sd->entity.pads[CSI2_PAD_SINK]); > - remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity); > + if (!remote_pad) > + return 0; > > - ipu6_isys_csi2_set_stream(sd, NULL, 0, false); > + remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity); > > v4l2_subdev_disable_streams(remote_sd, remote_pad->index, sink_streams); [Severity: High] This is a pre-existing issue, but does using remote_sd here without locks cause a similar use-after-free race in ipu6_isys_csi2_disable_streams()? If unbind completes before v4l2_subdev_disable_streams() finishes, the remote_sd pointer could be freed. v4l2_subdev_disable_streams() then dereferences sd->entity.graph_obj.mdev->dev. Could this lead to memory corruption or a kernel panic if a privileged user unbinds the device concurrently with stream stop? Also, as a pre-existing issue, does the IPU7 driver have the exact same missing NULL check that causes a panic? Looking at drivers/staging/media/ipu7/ipu7-isys-csi2.c in ipu7_isys_csi2_enable_streams() and ipu7_isys_csi2_disable_streams(): rp = media_pad_remote_pad_first(&sd->entity.pads[IPU7_CSI2_PAD_SINK]); r_sd = media_entity_to_v4l2_subdev(rp->entity); Since rp can be NULL if the sensor is unbound during capture, this will unconditionally dereference rp->entity and oops the kernel, just like the IPU6 driver did before this patch. Should this be fixed in the IPU7 driver as well? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911194854.78894-1-nicfio@gmail.com?part…

1 0

Re: [PATCH v2 2/3] media: v4l2-subdev: Check v4l2_dev before dereferencing it in open()
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [Critical] Highly exploitable Use-After-Free on the `v4l2_subdev` structure (`sd`) if the sensor driver uses devres and unbinds concurrently. - [High] Potential NULL pointer dereference when accessing `mdev->dev->driver->owner` due to a Time-Of-Check to Time-Of-Use (TOCTOU) race with device unbind. -- commit 67b3f0244ad4b5446e40eb46180000dfb338877c Author: Nicola Fiorillo <nicfio(a)gmail.com> media: v4l2-subdev: Check v4l2_dev before dereferencing it in open() The patch addresses a NULL pointer dereference that occurs when unbinding a sensor driver while its /dev/v4l-subdevN node is being opened. It adds checks for sd->v4l2_dev and sd->entity.graph_obj.mdev in subdev_open() because the device node can still be opened after these pointers are cleared by v4l2_device_unregister_subdev(). > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index e9f81b9be9e24..2a47b973097f4 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -97,8 +97,19 @@ static int subdev_open(struct file *file) > struct video_device *vdev = video_devdata(file); > struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev); > struct v4l2_subdev_fh *subdev_fh; > + struct v4l2_device *v4l2_dev; > int ret; > > + /* > + * v4l2_device_unregister_subdev() clears sd->v4l2_dev and unregisters > + * the entity before it unregisters the device node, so an open() that > + * races with the sub-device going away lands here with those pointers > + * already gone. > + */ > + v4l2_dev = READ_ONCE(sd->v4l2_dev); [Severity: Critical] This is a pre-existing issue, but does this code leave a use-after-free regression unaddressed? If subdev_open() races with v4l2_device_unregister_subdev(), and the sensor driver uses devres and unbinds concurrently, the driver core will execute devres cleanup (devres_release_all()) when remove() completes. This frees all devm_ managed memory, which typically embeds the v4l2_subdev structure. Because the video_device refcount taken by v4l2_open() does not prevent the driver core from freeing devm resources, if subdev_open() resumes after the sensor unbinds, READ_ONCE(sd->v4l2_dev) reads from freed memory. Furthermore, this freed pointer is later passed to subdev_fh_init(), which executes sd->internal_ops->init_state(), potentially allowing arbitrary code execution if the freed memory is overwritten. > + if (!v4l2_dev) > + return -ENODEV; > + > subdev_fh = kzalloc_obj(*subdev_fh); > if (subdev_fh == NULL) > return -ENOMEM; [ ... ] > @@ -112,15 +123,23 @@ static int subdev_open(struct file *file) > v4l2_fh_init(&subdev_fh->vfh, vdev); > v4l2_fh_add(&subdev_fh->vfh, file); > > - if (sd->v4l2_dev->mdev && sd->entity.graph_obj.mdev->dev) { > - struct module *owner; > + if (v4l2_dev->mdev) { > + struct media_device *mdev = READ_ONCE(sd->entity.graph_obj.mdev); > > - owner = sd->entity.graph_obj.mdev->dev->driver->owner; > - if (!try_module_get(owner)) { > - ret = -EBUSY; > + if (!mdev) { > + ret = -ENODEV; > goto err; > } > - subdev_fh->owner = owner; > + > + if (mdev->dev) { > + struct module *owner = mdev->dev->driver->owner; [Severity: High] This isn't a bug introduced by this patch, but can this dereference of mdev->dev->driver cause a NULL pointer dereference due to a race with device unbind? During unbind, the driver core calls __device_release_driver(), which unconditionally sets dev->driver = NULL. If subdev_open() evaluates mdev->dev->driver->owner without acquiring device_lock(mdev->dev) or another lock that serializes against the driver core, and is preempted after validating mdev->dev, couldn't mdev->dev->driver be NULL here? > + > + if (!try_module_get(owner)) { > + ret = -EBUSY; > + goto err; > + } > + subdev_fh->owner = owner; > + } > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911194854.78894-1-nicfio@gmail.com?part…

1 0

Re: [PATCH v2 3/3] media: ipu6: Signal the video queues when a sensor is unbound
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The newly added .unbind callback accesses queue state and modifies the q->error bitfield without holding the queue lock, introducing TOCTOU races and read-modify-write bitfield corruption. -- commit 494169b61b7502e0a85dfac1b24d10513720a639 Author: Nicola Fiorillo <nicfio(a)gmail.com> media: ipu6: Signal the video queues when a sensor is unbound This commit adds a missing .unbind() callback to isys_async_ops to notify ISYS video nodes when a sensor is removed. It marks streaming queues with an error flag to unblock vb2_core_dqbuf(), preventing an infinite wait when frames can no longer arrive. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys.c b/drivers/media/pci/intel/ipu6/ipu6-isys.c > index 24db2763de544..ec371ef9460fa 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys.c [ ... ] > @@ -700,6 +701,45 @@ static int isys_notifier_bound(struct v4l2_async_notifier *notifier, > return v4l2_device_register_subdev_nodes(&isys->v4l2_dev); > } > > +/* The .unbind() notifier callback when a sub-device goes away */ > +static void isys_notifier_unbind(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *sd, > + struct v4l2_async_connection *asc) > +{ [ ... ] > + csi2 = &isys->csi2[s_asd->csi2.port]; > + for (i = 0; i < NR_OF_CSI2_SRC_PADS; i++) { > + struct vb2_queue *q = &csi2->av[i].aq.vbq; > + > + if (!vb2_is_streaming(q)) > + continue; [Severity: High] Evaluating vb2_is_streaming(q) locklessly here may create a time-of-check to time-of-use (TOCTOU) race against concurrent STREAMON and STREAMOFF ioctls. Racing with STREAMON could cause the callback to skip setting the error flag just before the queue starts streaming, which would reintroduce the infinite hang. Racing with STREAMOFF could leave an idle queue permanently poisoned. Does this lockless read create a race condition that undermines the fix? > + > + dev_dbg(&isys->adev->auxdev.dev, > + "%s went away while streaming on %s\n", sd->name, > + csi2->av[i].vdev.name); > + vb2_queue_error(q); [Severity: High] vb2_queue_error(q) modifies the q->error bitfield locklessly. In struct vb2_queue, error, streaming, and waiting_in_dqbuf share the same bitfield memory word. Could a lockless update to q->error racing with a locked update (such as q->waiting_in_dqbuf = 1 in vb2_core_dqbuf) cause non-atomic read-modify-write tearing, leading to lost updates and silent state corruption? > + } > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911194854.78894-1-nicfio@gmail.com?part…

1 0

Re: [PATCH] media: marvell: manage the MMP camera clock provider
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: Pre-existing issues: - [Critical] Unconditional MMIO register access in shared IRQ handler without runtime PM clock enablement. - [High] Race condition exposing V4L2 device before hardware interrupt handler is requested. - [High] Devres Use-After-Free (UAF) due to freeing V4L2 structures while userspace holds open file descriptors. - [High] Null pointer dereference in subdevice pad operations due to bypass of V4L2 core wrappers. -- commit 8e5d8b68637f2729df170324ae1dc24992d14c39 Author: Myeonghun Pak <mhun512(a)gmail.com> media: marvell: manage the MMP camera clock provider mmpcam_probe() registers an OF clock provider before requesting the IRQ. Neither a later probe failure nor driver removal unregisters that provider, although the underlying clock is registered with devm_clk_register(). This leaves the provider referring to a clock whose lifetime has ended. > diff --git a/drivers/media/platform/marvell/mmp-driver.c b/drivers/media/platform/marvell/mmp-driver.c > index d3da7ebb4a2be..4c31b62751a12 100644 > --- a/drivers/media/platform/marvell/mmp-driver.c > +++ b/drivers/media/platform/marvell/mmp-driver.c [ ... ] > @@ -269,8 +270,8 @@ static int mmpcam_probe(struct platform_device *pdev) > /* > * Add OF clock provider. > */ > - ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get, > - mcam->mclk); > + ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_simple_get, > + &mcam->mclk_hw); [Severity: High] This isn't a bug introduced by this patch, but is there a race condition where the V4L2 device is exposed before the hardware interrupt handler is requested? In mmpcam_probe(), mccic_register() is called before the clock provider is added and before devm_request_irq() is called: drivers/media/platform/marvell/mmp-driver.c:mmpcam_probe() { ... ret = mccic_register(mcam); if (ret) ... /* Add OF clock provider */ ret = devm_of_clk_add_hw_provider(...); ... /* Finally, set up our IRQ ... */ ret = devm_request_irq(&pdev->dev, cam->irq, mmpcam_irq, IRQF_SHARED, ...); ... } Userspace can immediately open /dev/videoX and invoke VIDIOC_STREAMON, which activates hardware interrupts via mcam_ctlr_irq_enable(). Because the kernel has not yet registered the interrupt handler, the hardware could assert the shared IRQ line without a handler to acknowledge it, potentially causing an interrupt storm and denial of service for all devices sharing the interrupt. [Severity: Critical] This is a pre-existing issue, but could this code cause a synchronous external abort (bus fault) resulting in a kernel panic? mmpcam_probe() uses devm_request_irq() with IRQF_SHARED before the device is runtime-resumed (the device clocks are off). An interrupt from any other device sharing the same IRQ line immediately invokes mmpcam_irq(), which unconditionally reads a hardware register: drivers/media/platform/marvell/mmp-driver.c:mmpcam_irq() { ... irqs = mcam_reg_read(mcam, REG_IRQSTAT); ... } Because the clocks are gated by runtime PM, accessing the unclocked device register triggers a bus fault. [Severity: High] This is a pre-existing issue, but is there a devres use-after-free risk during driver removal? The V4L2 device cam structure is allocated using devres (devm_kzalloc) in mmpcam_probe(). During driver removal, mccic_shutdown() calls video_unregister_device(), which does not wait for existing userspace file descriptors to close: drivers/media/platform/marvell/mmp-driver.c:mmpcam_remove() { ... mccic_shutdown(mcam); v4l2_device_unregister(&mcam->v4l2_dev); ... } If userspace holds an open file descriptor to /dev/videoX when the driver is unbound, devres will immediately free the cam structure. A subsequent ioctl from userspace (like VIDIOC_QUERYCAP) would dereference freed memory, specifically cam->s_mutex via vdev->lock. [Severity: High] This is a pre-existing issue, but can this code cause a null pointer dereference when bypassing V4L2 core wrappers? The bridge driver directly invokes the subdevice's enum_frame_size and enum_frame_interval pad callbacks with a hardcoded NULL state pointer, instead of using the proper state-aware wrappers: drivers/media/platform/marvell/mcam-core.c:mcam_vidioc_enum_framesizes() { ... ret = sensor_call(cam, pad, enum_frame_size, NULL, &fse); ... } drivers/media/platform/marvell/mcam-core.c:mcam_vidioc_enum_frameintervals() { ... ret = sensor_call(cam, pad, enum_frame_interval, NULL, &fie); ... } If untrusted userspace calls VIDIOC_ENUM_FRAMESIZES or VIDIOC_ENUM_FRAMEINTERVALS, the subdevice expects a validated state and may dereference NULL when it attempts to look up formats via v4l2_subdev_state_get_format(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911175537.87996-1-mhun512@gmail.com?par…

1 0

Re: [v3,0/5] media: uvcvideo: Automatically handle invalid uvc_versions
by Patchwork Integration 11 Sep '26

11 Sep '26

Dear Ricardo Ribalda: Thanks for your patches! Unfortunately the Media CI robot detected some issues: # Test checkpatch:./0005-media-uvcvideo-Force-UVC-version-for-Avermedia-GC515.patch checkpatch WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #14: Bus 003 Device 021: ID 07ca:1515 AVerMedia Technologies, Inc. X'tra Go GC515 total: 0 errors, 1 warnings, 0 checks, 17 lines checked Please fix your series, and upload a new version. If you have a patchwork account, do not forget to mark the current series as Superseded. For more details, check the full report at: https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/11007610… . Best regards, and Happy Hacking! Media CI robot on behalf of the linux-media community. --- Check the latest rules for contributing your patches at: https://docs.kernel.org/driver-api/media/maintainer-entry-profile.html If you believe that the CI is wrong, kindly open an issue at https://gitlab.freedesktop.org/linux-media/media-ci/-/issues or reply-all to this message.

1 0

Re: [PATCH] media: rkisp1: Fix Bayer demosaicing bypass
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Incomplete fix for inverted BDM bypass logic in legacy API and teardown path -- commit 92c4700fb4f13edb78bd34be4fbd9d0cad86f4f2 Author: Jacopo Mondi <jacopo.mondi(a)ideasonboard.com> Subject: media: rkisp1: Fix Bayer demosaicing bypass The RKISP1_CIF_ISP_DEMOSAIC_BYPASS bit, when set, bypasses the demosaicing block on the RkISP1 ISP. The current implementation clears the bit when demosaicing has to be bypassed and sets it when demosaicing has to be enabled. This commit fixes the issue by setting the bypass bit when disabling the demosaicing block and clearing it when it has to be performed. > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > index 042b759eba625..496381962f1b4 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > @@ -1854,8 +1854,8 @@ rkisp1_ext_params_bdm(struct rkisp1_params *params, > const struct rkisp1_ext_params_bdm_config *bdm = &block->bdm; > > if (bdm->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE) { > - rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC, > - RKISP1_CIF_ISP_DEMOSAIC_BYPASS); > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC, > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS); > return; > } > > @@ -1863,8 +1863,8 @@ rkisp1_ext_params_bdm(struct rkisp1_params *params, > > if ((bdm->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE) && > !(params->enabled_blocks & BIT(bdm->header.type))) > - rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC, > - RKISP1_CIF_ISP_DEMOSAIC_BYPASS); > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC, > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS); > } [Severity: High] This isn't a bug introduced by this patch, but does the legacy parameters format handling also need to be updated? The exact same inverted bypass logic appears to remain in drivers/media/platform/rockchip/rkisp1/rkisp1-params.c when configuring BDM via the legacy V4L2_META_FMT_RK_ISP1_PARAMS format: rkisp1_isp_isr_other_config() { ... if (module_ens & RKISP1_CIF_ISP_MODULE_BDM) rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC, RKISP1_CIF_ISP_DEMOSAIC_BYPASS); else rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC, RKISP1_CIF_ISP_DEMOSAIC_BYPASS); ... } Additionally, could the pipeline teardown path have the same issue? Looking at rkisp1_params_disable(), it clears the bypass bit when stopping the stream: rkisp1_params_disable() { ... rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC, RKISP1_CIF_ISP_DEMOSAIC_BYPASS); ... } Does clearing this bit unintentionally enable the demosaic block while the rest of the modules are being disabled? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911-imx8mp-demosaicing-bypass-v1-1-5568…

1 0

Re: [PATCH v3 3/5] media: uvcvideo: Automatically handle cameras with invalid uvc_version
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [Critical] Deadlock and double URB submission when resuming from autosuspend during VIDIOC_STREAMON. -- commit 63ecbaf269d2fd11c61da15be773dda33873a644 Author: Ricardo Ribalda <ribalda(a)chromium.org> media: uvcvideo: Automatically handle cameras with invalid uvc_version Currently, the driver expects that cameras properly implement the spec version that they announce, and if they fail to do so, we do not continue probing the driver. This patch tries to handle these cameras with an identity crisis automatically. > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index e289cc71ba986..ca75f8d1ec46d 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c [ ... ] This is a pre-existing issue, but while reviewing the UVC driver, I noticed a potential deadlock and double URB submission when resuming from autosuspend during VIDIOC_STREAMON. In uvc_start_streaming_video(), which is called by the V4L2 core while holding the queue mutex and setting q->streaming = 1: drivers/media/usb/uvc/uvc_queue.c:uvc_start_streaming_video() { ... ret = uvc_pm_get(stream->dev); if (ret) goto err_buffers; queue->buf_used = 0; ret = uvc_video_start_streaming(stream); ... } [Severity: Critical] Does calling uvc_pm_get() here trigger a runtime resume while the vb2 queue is already marked as streaming? If so, during the resume process in uvc_video_resume(): drivers/media/usb/uvc/uvc_video.c:uvc_video_resume() { ... if (!uvc_queue_streaming(&stream->queue)) return 0; ret = uvc_commit_video(stream, &stream->ctrl); if (ret < 0) return ret; return uvc_video_start_transfer(stream, GFP_NOIO); ... } Can this prematurely call uvc_video_start_transfer() because the queue streaming state is already set to 1 before the device is fully resumed, leading to a duplicate URB allocation and submission? Furthermore, if uvc_video_resume() fails, the error recovery path in __uvc_resume() attempts to stop the queue: drivers/media/usb/uvc/uvc_driver.c:__uvc_resume() { ... ret = uvc_video_resume(stream, reset); if (ret < 0) { mutex_lock(&stream->queue.mutex); vb2_streamoff(&stream->queue.queue, stream->queue.queue.type); mutex_unlock(&stream->queue.mutex); } ... } Could this attempt to acquire the queue mutex result in a recursive deadlock, given that the mutex is already held by the V4L2 core during VIDIOC_STREAMON? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911-uvc-version-v3-0-604328d8a0dd@chrom…

1 0

Re: [PATCH] media: uvcvideo: Fix bounds for descriptor parsing
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Infinite loop in USB descriptor parsing due to unhandled zero-length descriptors - [Medium] Incomplete fix: Boundary overruns in Video Streaming descriptor parsers -- commit b0f11774a72d995c8523d2b6727051ab85efeb74 Author: Ricardo Ribalda <ribalda(a)chromium.org> Subject: media: uvcvideo: Fix bounds for descriptor parsing The commit changes uvc_parse_control() to pass the actual length of the descriptor (buffer[0]) rather than the remaining buffer length (buflen) to prevent malformed descriptors from leaking data over adjacent descriptors. It also adds a size validation check to ensure descriptors are valid. > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index e289cc71ba986..429f1ab19a2ac 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1248,11 +1248,14 @@ static int uvc_parse_control(struct uvc_device *dev) > */ > > while (buflen > 2) { > - if (uvc_parse_vendor_control(dev, buffer, buflen) || > + if (buflen < buffer[0] || buffer[0] < 3) > + return -EINVAL; > + [Severity: High] This is a pre-existing issue, but by adding this check for control descriptors, does it leave the video streaming descriptor parsers vulnerable to an infinite loop? If a malicious or malfunctioning USB device provides a Video Streaming descriptor with a bLength (buffer[0]) of 0, loops in uvc_parse_streaming() spin forever because the pointer and remaining length never change: /* Skip the standard interface descriptors. */ while (buflen > 2 && buffer[1] != USB_DT_CS_INTERFACE) { buflen -= buffer[0]; buffer += buffer[0]; } Similar unbounded loops exist in uvc_parse_streaming() when counting formats, and in uvc_parse_format() when iterating frames. > + if (uvc_parse_vendor_control(dev, buffer, buffer[0]) || > buffer[1] != USB_DT_CS_INTERFACE) > goto next_descriptor; > > - ret = uvc_parse_standard_control(dev, buffer, buflen); > + ret = uvc_parse_standard_control(dev, buffer, buffer[0]); [Severity: Medium] This isn't a bug introduced by this patch, but since we are fixing boundary overruns by using buffer[0] instead of buflen here, are the streaming descriptor parsers still vulnerable? In uvc_parse_streaming(), the total remaining buflen is passed to uvc_parse_format(): ret = uvc_parse_format(dev, streaming, format, frame, &interval, buffer, buflen); Inside uvc_parse_format() and uvc_parse_frame(), bounds checking evaluates the total buflen instead of the actual descriptor length (buffer[0]): n = buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED ? 27 : 28; if (buflen < n) { This allows a truncated streaming descriptor to pass bounds checks if the total remaining interface buffer length is large enough, leaking adjacent descriptor data into the parsing logic. > if (ret < 0) > return ret; > -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911-uvc-ctrl-bound-v1-1-7b5cfc68bae1@ch…

1 0

Re: [PATCH 1/3] firmware: qcom: pas: Expose the active PAS backend name
by sashiko-bot＠kernel.org 11 Sep '26

11 Sep '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] Returning a string pointer owned by a loadable backend module risks a Use-After-Free if the module is unloaded. Pre-existing issues: - [High] Time-of-check to time-of-use (TOCTOU) vulnerability on the global `ops_ptr` leading to potential NULL pointer dereference across PAS APIs. - [High] Concurrent registration of multiple PAS backends can cause one backend to silently overwrite the other due to a race condition. -- commit 9a78a0e63525decc94ed51a06493df2a2478414f Author: Jorge Ramirez-Ortiz <jorge.ramirez(a)oss.qualcomm.com> firmware: qcom: pas: Expose the active PAS backend name Consumers may need to know which PAS backend is active, e.g. to decide whether SCM-specific calls apply. This patch adds qcom_pas_get_backend() to return the active backend name, and defines backend constants. > diff --git a/drivers/firmware/qcom/qcom_pas.c b/drivers/firmware/qcom/qcom_pas.c > index 24485dd0fa10a..eb9e8ac858603 100644 > --- a/drivers/firmware/qcom/qcom_pas.c > +++ b/drivers/firmware/qcom/qcom_pas.c [ ... ] > @@ -253,6 +253,14 @@ bool qcom_pas_supported(u32 pas_id) > } > EXPORT_SYMBOL_GPL(qcom_pas_supported); > > +const char *qcom_pas_get_backend(void) > +{ > + struct qcom_pas_ops *ops = ops_ptr; [Severity: High] This is a pre-existing issue, but can the global ops_ptr be safely read here without READ_ONCE() or synchronization? This demonstrates a pattern present across PAS APIs, such as in qcom_pas_init_image(), where ops_ptr is checked against a null pointer and then dereferenced: if (!ops_ptr) return -ENODEV; return ops_ptr->init_image(...); Because ops_ptr is a global variable modified concurrently without locks (via smp_store_release() during unregister), the compiler can issue two loads. If the PAS backend module is unloaded between these two loads, the second load evaluates to a null pointer, causing a direct null pointer dereference. > + > + return ops ? ops->drv_name : NULL; [Severity: High] Is there a risk of a use-after-free if the backend module is unloaded? qcom_pas_get_backend() returns ops->drv_name. For the TEE backend, this points to a string literal ("qcom-pas-tee") residing in the .rodata section of the loadable module qcom_pas_tee.ko. If a consumer like the iris driver calls this function and uses the returned pointer in strcmp(), and qcom_pas_tee is unloaded via rmmod between the pointer being returned and strcmp() executing, the module memory is freed. This would cause strcmp() to read unmapped or reallocated memory. > +} > +EXPORT_SYMBOL_GPL(qcom_pas_get_backend); While reviewing the global ops_ptr usage, I noticed a pre-existing issue in qcom_pas_ops_register() in drivers/firmware/qcom/qcom_pas.c: > void qcom_pas_ops_register(struct qcom_pas_ops *ops) > { > if (!qcom_pas_is_available()) > /* Paired with smp_load_acquire() in qcom_pas_is_available() */ > smp_store_release(&ops_ptr, ops); > else > pr_err("qcom_pas: ops already registered by %s\n", > ops_ptr->drv_name); > } [Severity: High] This isn't a bug introduced by this patch, but could concurrent registration of multiple PAS backends cause one backend to silently overwrite the other? qcom_pas_ops_register() verifies if a backend is already registered by calling qcom_pas_is_available() (which checks ops_ptr), and then writes the new pointer using smp_store_release(). Because there is no mutex protecting this check-and-update sequence, two PAS backend modules (like SCM and TEE) probing concurrently can both observe a null ops_ptr and proceed to register. The second registration silently overwrites the first, leaving the first module believing it is successfully registered when it is not. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260911-video-optee-pas-v1-v1-0-7bbddaaac5b…

1 0

2026

media-ci September 2026 ----- 2026 ----- September 2026 August 2026 July 2026 June 2026 May 2026 April 2026 March 2026 February 2026 January 2026

media-ci September 2026