October 2026 - media-ci

Re: [PATCH 5/5] media: ipu6: Return errors from stream stop and close
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] media: out-of-bounds write via unvalidated stream->vc - [Medium] media: ipu6: Early return in ipu6_isys_stop_stream_firmware() bypasses need_reset marking - [Low] media: ipu6: Primary error code overwritten during cleanup path in ipu6_isys_start_stream_firmware() -- commit 420d76dca7d258f867a9b52783e95225a692f015 Author: Sakari Ailus <sakari.ailus(a)linux.intel.com> media: ipu6: Return errors from stream stop and close If the firmware fails to start, stop or close a stream, either by telling about it with an error or due to a timeout, it probably requires a reset before being operational again. Mark isys needing a reset in such cases. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > index 44541aff79b30..799a4be3160b5 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > @@ -543,25 +543,31 @@ int ipu6_isys_start_stream_firmware(struct ipu6_isys_stream *stream, > out_stream_close: > reinit_completion(&stream->stream_close_completion); > > + scoped_guard(mutex, &stream->isys->mutex) > + stream->isys->need_reset = true; > + > retout = fw_ops->stream_close(stream->isys, stream->stream_handle); > if (retout < 0) { > dev_dbg(dev, "can't close stream (%d)\n", retout); > - return retout; > + return ret; > } > > tout = wait_for_completion_timeout(&stream->stream_close_completion, > IPU6_FW_CALL_TIMEOUT_JIFFIES); > - if (!tout) > + if (!tout) { > dev_err(dev, "stream close time out\n"); > - else if (stream->error) > + ret = -ETIMEDOUT; > + } else if (stream->error) { > dev_err(dev, "stream close error: %d\n", stream->error); > - else > + ret = -EINVAL; > + } else { > dev_dbg(dev, "stream close complete\n"); > + } [Severity: Low] Does this code inadvertently clobber the primary stream start error code? If fw_ops->stream_start() fails, execution jumps to the out_stream_close label with ret holding the primary error code. If the subsequent firmware close command times out or encounters a stream error, the newly added braces update ret to -ETIMEDOUT or -EINVAL, overwriting the original error before returning it to the caller. > > return ret; > } > > -void ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream) > +int ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream) > { > struct ipu6_bus_device *adev = stream->asd->isys->adev; > const struct ipu6_fw_isys_ops *fw_ops = adev->auxdrv_data->fw_ops; > @@ -573,20 +579,30 @@ void ipu6_isys_stop_stream_firmware(struct ipu6_isys_stream *stream) > ret = fw_ops->stream_flush(stream->isys, stream->stream_handle); > if (ret < 0) { > dev_err(dev, "can't stop stream (%d)\n", ret); > - return; > + return -EBUSY; [Severity: Medium] Does this early return inadvertently bypass marking the device for a reset? If fw_ops->stream_flush() returns an error, this path returns -EBUSY and completely skips the newly added logic at the end of the function: if (ret) { scoped_guard(mutex, &stream->isys->mutex) stream->isys->need_reset = true; } Because this skips the reset logic, it appears to contradict the commit message's stated intent to mark the device for reset when firmware fails to stop a stream. > } > > tout = wait_for_completion_timeout(&stream->stream_stop_completion, > IPU6_FW_CALL_TIMEOUT_JIFFIES); [ ... ] > @@ -599,22 +615,33 @@ void ipu6_isys_close_stream_firmware(struct ipu6_isys_stream *stream) > ret = fw_ops->stream_close(stream->isys, stream->stream_handle); > if (ret < 0) { > dev_err(dev, "can't close stream (%d)\n", ret); > - return; > + goto out_cleanup; > } > > tout = wait_for_completion_timeout(&stream->stream_close_completion, > IPU6_FW_CALL_TIMEOUT_JIFFIES); > - if (!tout) > + if (!tout) { > dev_warn(dev, "stream close time out\n"); > - else if (stream->error) > + ret = -ETIMEDOUT; > + } else if (stream->error) { > dev_warn(dev, "stream close error: %d\n", stream->error); > - else > + ret = -EINVAL; > + } else { > dev_dbg(dev, "close stream: complete\n"); > + } > > +out_cleanup: > scoped_guard(spinlock_irqsave, &stream->isys->streams_lock) { > stream->isys->streams_by_handle[stream->stream_handle] = NULL; > csi2->streams_by_vc[stream->vc] = NULL; > } [Severity: High] Does this error path introduce an out-of-bounds memory write if a malicious or buggy sensor provides an invalid vc value? The stream->vc value originates from an external subdevice (sensor) driver's get_frame_desc operation. Because the V4L2 core does not enforce bounds on vc, the value can exceed NR_OF_CSI2_VC (16). If a stream close fails and execution jumps to the out_cleanup label: csi2->streams_by_vc[stream->vc] = NULL; an unbounded vc value will overwrite subsequent fields in the ipu6_isys_csi2 struct, such as the base iomem pointer. > + > + if (ret) { > + scoped_guard(mutex, &stream->isys->mutex) > + stream->isys->need_reset = true; > + } > + > + return ret; > } > > struct ipu6_isys_stream * -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008085247.798156-1-sakari.ailus@linux.i…

1 0

Re: [PATCH 4/5] media: ipu6: Remove need_reset check from video device open
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: ipu6: Buffer leak in start_streaming error path bypassing out_return_buffers -- commit 05f370625cb57787ffdbece5abefcb5044316ff0 Author: Sakari Ailus <sakari.ailus(a)linux.intel.com> media: ipu6: Remove need_reset check from video device open The ISYS requires power reset in some cases. Opening a video node is however independent of that; allow opening it even if reset is needed. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > index 129016e57446b..44541aff79b30 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c > @@ -109,23 +109,6 @@ const struct ipu6_isys_pixelformat ipu6_isys_pfmts[] = { > IPU6_FW_ISYS_FRAME_FORMAT_RAW16, true }, > }; > > -static int video_open(struct file *file) > -{ > - struct ipu6_isys_video *av = video_drvdata(file); > - struct ipu6_isys *isys = av->isys; > - struct ipu6_bus_device *adev = isys->adev; > - > - mutex_lock(&isys->mutex); > - if (isys->need_reset) { > - mutex_unlock(&isys->mutex); > - dev_warn(&adev->auxdev.dev, "isys power cycle required\n"); > - return -EIO; > - } > - mutex_unlock(&isys->mutex); > - > - return v4l2_fh_open(file); > -} > - [ ... ] > @@ -809,7 +792,7 @@ static const struct v4l2_file_operations isys_fops = { > .poll = vb2_fop_poll, > .unlocked_ioctl = video_ioctl2, > .mmap = vb2_fop_mmap, > - .open = video_open, > + .open = v4l2_fh_open, [Severity: High] Does removing the need_reset check from open expose a buffer leak in the start_streaming error path? If the hardware requires a reset (isys->need_reset is true) when the user queues buffers via VIDIOC_QBUF, the buffers are added to aq->incoming. When the user then calls VIDIOC_STREAMON, start_streaming() in drivers/media/pci/intel/ipu6/ipu6-isys-queue.c returns directly without cleaning up the queued buffers: scoped_guard(mutex, &av->isys->mutex) { if (av->isys->need_reset) return -EIO; } Because this direct return bypasses the goto out_return_buffers error path, it skips returning the active buffers. Does this leave the queued buffers orphaned in aq->incoming, resulting in a videobuf2 core warning and a use-after-free on subsequent queue operations or stream restart? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008085247.798156-1-sakari.ailus@linux.i…

1 0

Re: [PATCH 2/5] media: ipu6: Tell about not being able to obtain frame descriptor
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: ipu6: resource leaks and list corruption on get_frame_desc failure -- commit ab555e23d2d582e2dd0f99a4cb6584f408d23edc Author: Sakari Ailus <sakari.ailus(a)linux.intel.com> media: ipu6: Tell about not being able to obtain frame descriptor Not being able to obtain a frame descriptor is a fatal error at this point. The frame descriptor should be stored for the duration of the streaming operation. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > index 1525454279308..3e9bfda2a8b9a 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > @@ -749,8 +749,10 @@ static int ipu6_isys_csi2_disable_streams(struct v4l2_subdev *sd, > lockdep_assert_held(&csi2->isys->stream_mutex); > > ret = ipu6_isys_get_frame_desc(remote_sd, remote_pad->index, &desc); > - if (ret) > - return ret; > + if (ret) { > + dev_err(sd->dev, "cannot obtain frame descriptor\n"); > + return 0; > + } [Severity: High] Does returning 0 here in ipu6_isys_csi2_disable_streams() instead of an error code mask the failure from the V4L2 core, leading to resource leaks and list corruption? By returning 0, the V4L2 core updates its state to mark the stream as stopped, even though the hardware disablement and teardown steps have been bypassed. This early return skips important cleanup such as pm_runtime_put(sd->dev), which leaks a PM reference count. It also skips list_del(&av->csi2_entry), leaving the video device's list entry linked in the csi2 list. If a subsequent attempt is made to enable the stream, it will call list_add() on the already-linked node, which can corrupt the doubly-linked list. If the video device is freed, it could also result in a use-after-free when traversing the list. While failing to clean up on error was preexisting, changing this to return 0 masks the failure and incorrectly signals success to the V4L2 core without fixing the teardown path. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008085247.798156-1-sakari.ailus@linux.i…

1 0

Re: [PATCH 3/5] media: ipu6: Prevent starting streaming if ISYS needs reset
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: buffer leak in start_streaming due to bypassed goto cleanup -- commit 5bdfbd6c853742afe356643c25551aec647f2e92 Author: Sakari Ailus <sakari.ailus(a)linux.intel.com> media: ipu6: Prevent starting streaming if ISYS needs reset Fail starting streaming whenever ISYS needs to be reset to resume normal operation. > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c > index 6596833e95d49..978acbd72e63f 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c > @@ -481,6 +481,11 @@ static int start_streaming(struct vb2_queue *q, unsigned int count) > av->vdev.name, ipu6_isys_get_frame_width(av), > ipu6_isys_get_frame_height(av), pfmt->css_pixelformat); > > + scoped_guard(mutex, &av->isys->mutex) { > + if (av->isys->need_reset) > + return -EIO; [Severity: High] Does this direct return bypass the required buffer cleanup? The start_streaming videobuf2 callback requires that if an error is returned, any buffers previously queued to the driver must be explicitly returned. The existing error paths handle this by jumping to out_return_buffers, which calls return_buffers(aq, VB2_BUF_STATE_QUEUED). Could returning directly here leave these buffers stranded on the driver's internal incoming list? Mixing scoped_guard with goto-based cleanup in the same function can easily cause resource leaks like this. > + } > + > remote_pad = media_pad_remote_pad_unique(&av->pad); > if (IS_ERR(remote_pad)) { > dev_dbg(dev, "failed to get remote pad\n"); -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008085247.798156-1-sakari.ailus@linux.i…

1 0

Re: [PATCH v4 3/3] media: qcom: camss: Add support for X1P42100 CAMSS
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] media: camss: mismatched CSID to VFE indices break power on - [High] media: camss: null dereference during probe with malformed DT on X1P42100 - [High] media: camss: missing bounds check on DT port number leads to OOB write -- commit ab38e0eb97dc4d60c438b6b2e2fd2590708e04a4 Author: Wenmeng Liu <wenmeng.liu(a)oss.qualcomm.com> media: qcom: camss: Add support for X1P42100 CAMSS This commit adds CAMSS support for the Qualcomm X1P42100 (Purwa) SoC. It defines the X1P42100 hardware configuration, reusing existing CSID/VFE ops while omitting unused components like CSIPHY1-3 and one IFE. > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index c54185c93a8a9..7bc6b2e61438d 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -4452,6 +4452,205 @@ static const struct resources_wrapper csid_wrapper_res_x1e80100 = { > .reg = "csid_wrapper", > }; > > +static const struct camss_subdev_resources csiphy_res_x1p42100[] = { > + /* CSIPHY0 */ > + { > + .csiphy = { > + .id = 0, > + .hw_ops = &csiphy_ops_3ph_1_0, > + .formats = &csiphy_formats_sdm845 > + }, > + }, [Severity: High] If camss_detect_legacy_phy() falls back to legacy_phy = true due to a malformed device tree without a phy remote endpoint, camss_init_subdevices() invokes msm_csiphy_subdev_init_legacy(). That function attempts to map memory using res->reg[0], but since this structure does not define .reg, it passes a NULL pointer to platform_get_resource_byname() which then crashes in strcmp(). Additionally, csiphy_init() lacks a case for CAMSS_X1P42100, which leaves regs->lane_regs uninitialized in this fallback path. Will this cause a NULL pointer dereference during driver probe? > + /* CSIPHY4 */ > + { > + .csiphy = { > + .id = 4, > + .hw_ops = &csiphy_ops_3ph_1_0, > + .formats = &csiphy_formats_sdm845 > + }, > + }, > +}; [Severity: High] When parsing endpoints from the device tree, camss_parse_endpoint_node() assigns the device tree port number directly to csd->interface.csiphy_id without bounds checking against csiphy_num. If port 4 is used in the device tree to match this hardware ID, csiphy_id becomes 4. Later, camss_subdev_notifier_bound() uses this unvalidated id to index into the camss->csiphy array. Since that array only has 2 elements allocated for X1P42100, could defining this with id 4 lead to an out-of-bounds memory overwrite during subdevice binding? [ ... ] > +static const struct camss_resources x1p42100_resources = { > + .version = CAMSS_X1P42100, > + .pd_name = "top", > + .csiphy_res = csiphy_res_x1p42100, > + .tpg_res = tpg_res_x1e80100, > + .csid_res = csid_res_x1p42100, > + .vfe_res = vfe_res_x1p42100, > + .csid_wrapper_res = &csid_wrapper_res_x1e80100, > + .icc_res = icc_res_x1p42100, > + .icc_path_num = ARRAY_SIZE(icc_res_x1p42100), > + .csiphy_num = ARRAY_SIZE(csiphy_res_x1p42100), > + .tpg_num = ARRAY_SIZE(tpg_res_x1e80100), > + .csid_num = ARRAY_SIZE(csid_res_x1p42100), > + .vfe_num = ARRAY_SIZE(vfe_res_x1p42100), > +}; [Severity: High] During initialization, csid->id is assigned the index from the resource array (0 to 4). When csid_set_power() attempts to power on its paired VFE using its own ID, it calls vfe_parent_dev_ops_get() which checks if id < camss->res->vfe_num. For IDs 3 and 4 (CSID_LITE0 and CSID_LITE1), this bounds check fails and unconditionally returns -EINVAL, aborting the power-up. Additionally, this 1:1 index pairing appears to cause CSID1 and CSID2 to incorrectly power up IFE_LITE_0 and IFE_LITE_1 instead of their expected components. Does the mismatch between the number of CSIDs (5) and VFEs (3) break the power-on sequence for the CSID blocks? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008-purwa_camss-v4-0-bcf11bf2c650@oss.q…

1 0

Re: [v19,00/12] media: qcom: camss: Add Kaanapali support
by Patchwork Integration 08 Oct '26

08 Oct '26

Dear Hangxiang Ma: Thanks for your patches! Unfortunately the Media CI robot detected some issues: # Test checkpatch:./0004-media-dt-bindings-Add-CAMSS-device-for-Kaanapali.patch checkpatch WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #27: new file mode 100644 total: 0 errors, 1 warnings, 0 checks, 303 lines checked # Test checkpatch:./0007-media-qcom-camss-csid-Add-support-for-CSID-1080.patch checkpatch WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #42: new file mode 100644 CHECK: Macro argument reuse 'rdi' - possible side-effects? #120: FILE: drivers/media/platform/qcom/camss/camss-csid-1080.c:74: +#define CSID_RDI_CFG0(rdi)(csid_is_lite(csid) ?\ +(0x3080 + 0x200 * (rdi)) :\ +(0x5480 + 0x200 * (rdi))) CHECK: Macro argument reuse 'rdi' - possible side-effects? #132: FILE: drivers/media/platform/qcom/camss/camss-csid-1080.c:86: +#define CSID_RDI_CTRL(rdi)(csid_is_lite(csid) ?\ +(0x3088 + 0x200 * (rdi)) :\ +(0x5488 + 0x200 * (rdi))) CHECK: Macro argument reuse 'rdi' - possible side-effects? #137: FILE: drivers/media/platform/qcom/camss/camss-csid-1080.c:91: +#define CSID_RDI_CFG1(rdi)(csid_is_lite(csid) ?\ +(0x3094 + 0x200 * (rdi)) :\ +(0x5494 + 0x200 * (rdi))) total: 0 errors, 1 warnings, 3 checks, 502 lines checked # Test checkpatch:./0008-media-qcom-camss-vfe-Add-support-for-VFE-1080.patch checkpatch WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #50: new file mode 100644 total: 0 errors, 1 warnings, 0 checks, 420 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/11208343… . 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 v19 10/12] arm64: dts: qcom: kaanapali: Add CAMSS and CSIPHY block definitions
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] dts: missing SoC-internal OF graph links between camss and standalone csiphy nodes -- commit a63685b077e80fcbb61d58b04d437addeb277a93 Author: Hangxiang Ma <hangxiang.ma(a)oss.qualcomm.com> arm64: dts: qcom: kaanapali: Add CAMSS and CSIPHY block definitions This commit describes the CAMSS and CSIPHY blocks on the Kaanapali SoC. It adds 6 CSIPHYs, 3 TPGs, 3 CSIDs, 2 CSID Lites, 3 VFEs, and 2 VFE Lites to enable camera pipelines on board device trees. > diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi > index e48a179d07220..7e5106865e7ee 100644 > --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi > +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi [ ... ] > @@ -3597,6 +3598,307 @@ usb_dp_qmpphy_dp_in: endpoint { > }; > }; > > + camss: isp@9253000 { > + compatible = "qcom,kaanapali-camss"; [ ... ] > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + }; > + > + port@1 { > + reg = <1>; > + }; [Severity: High] Will the CAMSS driver be able to find the CSIPHY devices with these empty port definitions? The CAMSS driver (msm_csiphy_subdev_init in camss-csiphy.c) discovers its PHYs by traversing the OF graph using of_graph_get_endpoint_by_regs on the camss node's ports, following it to the remote endpoint's parent, and calling devm_phy_get_by_of_node. Because these ports lack endpoint definitions linking back to the CSIPHY outputs, the lookup will return NULL and silently skip PHY initialization. This leaves the phy uninitialized and prevents the PHYs from powering on during streaming. [ ... ] > + }; > + }; > + > + csiphy0: phy@9523000 { > + compatible = "qcom,kaanapali-csi2-phy"; > + reg = <0x0 0x09523000 0x0 0x2000>; > + > + clocks = <&camcc CAM_CC_CSIPHY0_CLK>, [Severity: High] Does this standalone csiphy0 node also need a ports structure to define its CAMSS output endpoint? Without a ports definition in the csiphy nodes, the OF graph traversal from the camss node cannot resolve the remote endpoint, breaking the internal SoC routing required to initialize the PHYs. Since these are hardwired SoC-internal connections between the PHY output and the CAMSS input, shouldn't they be defined here in the .dtsi rather than forcing downstream board .dts files to manually construct the internal SoC routing? [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008-kaanapali-camss-v19-0-b2c583ef50e0@…

1 0

Re: [PATCH v19 08/12] media: qcom: camss: vfe: Add support for VFE 1080
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] media: camss: Hardware constraint violation on AUP_UPDATE count - [High] media: camss: Hardcoded VFE-to-CSID mapping breaks dynamic media routing - [High] media: camss: Data race on csid->aup_update and csid->rup_update -- commit f4a9516b2392f133fd18f4aed515b9cb62df16ca Author: Hangxiang Ma <hangxiang.ma(a)oss.qualcomm.com> media: qcom: camss: vfe: Add support for VFE 1080 Add support for the VFE (Video Front End) 1080 found on Kaanapali SoC. In the Kaanapali camera subsystem, the front-end blocks are referred to as TFEs. > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-1080.c b/drivers/media/platform/qcom/camss/camss-vfe-1080.c > new file mode 100644 > index 0000000000000..016857cd10251 > --- /dev/null > +++ b/drivers/media/platform/qcom/camss/camss-vfe-1080.c [ ... ] > +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id) > +{ > + int port_id = line_id; > + > + camss_reg_update(vfe->camss, vfe->id, port_id, false); [Severity: High] Does hardcoding vfe->id introduce a regression for dynamic media routing? The Media Controller framework allows dynamic routing, where any CSID source could be linked to any VFE sink (e.g., CSID[0] to VFE[2]). Here, camss_reg_update() is called with vfe->id, which indexes the CSID array directly: camss.c:camss_reg_update() { csid = &camss->csid[hw_id]; csid->res->hw_ops->reg_update(csid, port_id, is_clear); } If CSID[0] is routed to VFE[2], the VFE buffer queue will update the registers of CSID[2] instead of the active CSID[0], leaving the active CSID without its AUP_UPDATE and stalling the stream. [Severity: High] Can this introduce a data race regression on the CSID shadow states? When userspace queues new buffers via vfe_queue_buffer_v2(), it executes from process context and eventually calls this vfe_reg_update(). This ends up in __csid_aup_update() which performs an unprotected read-modify-write: camss-csid-1080.c:__csid_aup_update() { csid->aup_update |= CSID_RUP_AUP_RDI(port_id); } At the same time, the CSID hardware interrupt handler csid_isr() processes completions and performs a similar unprotected read-modify-write: camss-csid-1080.c:__csid_aup_rup_clear() { csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id); } Since these modifications can interleave without locks, one CPU's state update could be lost, potentially corrupting the CSID_AUP_CMD register state. > +} [ ... ] > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c > index 2680a9914765f..3acef6ba97dc9 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c [ ... ] > @@ -552,7 +554,9 @@ int vfe_enable_output_v2(struct vfe_line *line) > output->gen2.active_num++; > ops->vfe_wm_update(vfe, output->wm_idx[0], > output->buf[i]->addr[0], line); > - ops->reg_update(vfe, line->id); > + > + if (!vfe->res->reg_update_after_csid_config) > + ops->reg_update(vfe, line->id); > } [Severity: High] Does this introduce a regression by violating the AUP_UPDATE hardware constraint? The commit message states that "the number of AUP_UPDATE requests must match the number of buffers queued to the write master while it is being enabled." However, if userspace only queued 1 buffer, the loop in vfe_enable_output_v2() will break early (as it loops up to CAMSS_INIT_BUF_COUNT): for (i = 0; i < CAMSS_INIT_BUF_COUNT; i++) { output->buf[i] = vfe_buf_get_pending(output); if (!output->buf[i]) break; This means the VFE write master gets configured with only 1 buffer. Meanwhile, csid_configure_stream() in camss-csid-1080.c unconditionally issues CAMSS_INIT_BUF_COUNT updates: for (k = 0; k < CAMSS_INIT_BUF_COUNT; k++) __csid_aup_update(csid, i); If the CSID sends 2 AUP_UPDATE commands but the VFE is only configured with 1 buffer, can this cause the hardware to perform out-of-bounds DMA or trigger an IOMMU fault? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008-kaanapali-camss-v19-0-b2c583ef50e0@…

1 0

Re: [PATCH v19 07/12] media: qcom: camss: csid: Add support for CSID 1080
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] media: data race on aup_update and rup_update masks between process and IRQ context - [High] media: missing IRQ mask programming prevents INFO_RUP_DONE interrupts - [Medium] media: camss: csid-1080: data race on aup_update and rup_update bitmasks -- commit c1ffc650d77cc9e495d5b9cec2dfa9e85e2f485e Author: Hangxiang Ma <hangxiang.ma(a)oss.qualcomm.com> media: qcom: camss: csid: Add support for CSID 1080 Add support for the CSID 1080 hardware found on Kaanapali SoC. Introduce CSID 1080 hardware resource descriptions and implement the hardware-specific register programming, reset sequence, and BUF_DONE interrupt handling. > diff --git a/drivers/media/platform/qcom/camss/camss-csid-1080.c b/drivers/media/platform/qcom/camss/camss-csid-1080.c > new file mode 100644 > index 0000000000000..46e443e05195a > --- /dev/null > +++ b/drivers/media/platform/qcom/camss/camss-csid-1080.c [ ... ] > +static void __csid_aup_rup_clear(struct csid_device *csid, int port_id) > +{ > + /* Hardware clears the registers upon consuming the settings */ > + csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id); > + csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id); > +} > + > +static void __csid_aup_update(struct csid_device *csid, int port_id) > +{ > + csid->aup_update |= CSID_RUP_AUP_RDI(port_id); > + writel(csid->aup_update, csid->base + CSID_AUP_CMD); > + > + __csid_aup_rup_trigger(csid); > +} > + > +static void __csid_reg_update(struct csid_device *csid, int port_id) > +{ > + csid->rup_update |= CSID_RUP_AUP_RDI(port_id); > + writel(csid->rup_update, csid->base + CSID_RUP_CMD); > + > + __csid_aup_rup_trigger(csid); > +} [Severity: High] Does this code introduce a data race between process and hard IRQ context? In process context, csid_configure_stream() updates these variables via non-atomic bitwise OR (|=) using __csid_aup_update() and __csid_reg_update(). Concurrently, the hardware interrupt handler csid_isr() (running in hard IRQ context) clears bits in these exact same variables via non-atomic bitwise AND NOT (&= ~) through __csid_aup_rup_clear(). Because there is no spinlock or atomic bit operations protecting these read-modify-write sequences, an interrupt firing on the same or another CPU during stream configuration could cause a TOCTOU data race. Could this lead to lost bit updates, resulting in hardware commands being dropped or spuriously re-triggered? [ ... ] > +static irqreturn_t csid_isr(int irq, void *dev) > +{ > + struct csid_device *csid = dev; > + u32 val, buf_done_val; > + u8 reset_done; > + int i; > + > + val = readl(csid->base + CSID_TOP_IRQ_STATUS); > + writel(val, csid->base + CSID_TOP_IRQ_CLEAR); > + > + reset_done = val & INFO_RST_DONE; > + > + buf_done_val = readl(csid->base + CSID_BUF_DONE_IRQ_STATUS); > + writel(buf_done_val, csid->base + CSID_BUF_DONE_IRQ_CLEAR); > + > + for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_1080; i++) { > + if (csid->phy.en_vc & BIT(i)) { > + val = readl(csid->base + CSID_CSI2_RDIN_IRQ_STATUS(i)); > + writel(val, csid->base + CSID_CSI2_RDIN_IRQ_CLEAR(i)); > + > + if (val & INFO_RUP_DONE) > + csid_subdev_reg_update(csid, i, true); [Severity: High] Is the INFO_RUP_DONE interrupt properly unmasked? The driver relies on the INFO_RUP_DONE interrupt to clear the pending aup_update mask in the ISR via csid_subdev_reg_update(). However, the driver never writes to CSID_CSI2_RDIN_IRQ_MASK(i) to unmask these interrupts, nor does it enable the corresponding top-level aggregation bit (CSI2_RX_IRQ_STATUS) in CSID_TOP_IRQ_MASK during setup or reset. Because the interrupts appear to be permanently masked in hardware, the RUP_DONE interrupt might never trigger the csid_isr() independently. Could the aup_update state be left orphaned if no BUF_DONE interrupt happens to fire, breaking asynchronous updates? [ ... ] > +static int csid_reset(struct csid_device *csid) > +{ > + unsigned long time; > + u32 val; > + int i; > + > + reinit_completion(&csid->reset_complete); > + > + val = INFO_RST_DONE | BUF_DONE_IRQ_STATUS; > + writel(val, csid->base + CSID_TOP_IRQ_CLEAR); > + writel(val, csid->base + CSID_TOP_IRQ_MASK); > + > + val = 0; > + for (i = 0; i < MSM_CSID_MAX_SRC_STREAMS_1080; i++) { > + if (csid->phy.en_vc & BIT(i)) { > + /* > + * Only need to clear buf done IRQ status here, > + * RUP done IRQ status will be cleared once isr > + * strobe generated by CSID_RST_CMD > + */ > + val |= BIT(BUF_DONE_IRQ_STATUS_RDI_OFFSET + i); > + } > + } > + writel(val, csid->base + CSID_BUF_DONE_IRQ_CLEAR); > + writel(val, csid->base + CSID_BUF_DONE_IRQ_MASK); [Severity: High] This appears to be where top-level masking explicitly omits CSI2_RX_IRQ_STATUS, and CSID_CSI2_RDIN_IRQ_MASK is never programmed, as noted above. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008-kaanapali-camss-v19-0-b2c583ef50e0@…

1 0

Re: [PATCH v19 06/12] media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY
by sashiko-bot＠kernel.org 08 Oct '26

08 Oct '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] media: camss: NULL dereference in msm_csiphy_subdev_init_legacy on Kaanapali - [Medium] doc: commit message claims to modify DPHY driver but patch diff only modifies camss.c -- commit ee4f5ef31347740ffd64c4c9ec08e7e1f2398bfc Author: Hangxiang Ma <hangxiang.ma(a)oss.qualcomm.com> media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY This commit adds support for the v2.4.0 two-phase CSIPHY found on Kaanapali, defining a new resource structure for its configuration. > Add support for the v2.4.0 two-phase CSIPHY found on Kaanapali, where > the PHY is driven by the standalone phy-qcom-mipi-csi2 DPHY driver > rather than by CAMSS. Add the mipi_csi2_dphy_3nm_kaanapali configuration, > which reuses the x1e80100 lane sequence, clocks and supplies but selects > the v2.4.0 common status offset, adds Kaanapali power domain and register > the "qcom,kaanapali-csi2-phy" compatible. [Severity: Medium] Does this patch omit some files, or does the commit message incorrectly attribute changes that belong to another patch in the series? The commit message explicitly claims to add the DPHY configuration and register the compatible, but the actual patch diff only modifies drivers/media/platform/qcom/camss/camss.c. > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index fca3de214a922..eeceb79c1e863 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -38,6 +38,51 @@ > > static const struct parent_dev_ops vfe_parent_dev_ops; > > +static const struct camss_subdev_resources csiphy_res_kaanapali[] = { > + /* CSIPHY0 */ > + { > + .csiphy = { [Severity: High] Does this code cause a NULL pointer dereference on probe if the device tree lacks endpoints for the camss node? If a malformed or empty device tree is provided, camss_detect_legacy_phy() leaves camss->legacy_phy as true. During camss_init_subdevices(), this triggers a fallback to the legacy initialization: camss_init_subdevices() -> msm_csiphy_subdev_init_legacy() The legacy initialization unconditionally dereferences hw_ops: ret = csiphy->res->hw_ops->init(csiphy); Since csiphy_res_kaanapali omits hw_ops here, wouldn't this result in an immediate kernel panic instead of a clean error during probe? > + .id = 0, > + .formats = &csiphy_formats_sdm845 > + }, > + }, > + /* CSIPHY1 */ > + { [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20261008-kaanapali-camss-v19-0-b2c583ef50e0@…

1 0

2026

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

media-ci October 2026