August 2026 - media-ci

Re: [PATCH v4 4/4] media: imagination: e5010: Enable autosuspend for runtime PM
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: New issues: - [High] If pm_runtime_resume_and_get() fails in e5010_device_run(), the function finishes the job but fails to dequeue the ready buffers, causing an infinite scheduling loop in the V4L2 mem2mem framework. - [High] Unbalanced clock disable in e5010_remove() due to unconditional disable on a potentially suspended device. Pre-existing issues: - [High] video_register_device() is called before runtime PM is enabled in e5010_probe(), exposing the device to userspace prematurely and creating a race condition that triggers the infinite loop. - [High] Use-after-free of e5010 context in e5010_release() on late file close due to devres allocation. - [High] Use-after-free of m2m_dev in e5010_irq() due to concurrent execution during driver unbind. -- commit f2a58df9e7781a157128a3c705c5c2953ea6e6f9 Author: Brandon Brnich <b-brnich(a)ti.com> Subject: media: imagination: e5010: Enable autosuspend for runtime PM This commit enables autosuspend to save power instead of only keeping the device on during STREAM_ON. It also sets last_context_run to NULL on resume to trigger QP value updates. > diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c > index 488f5d7e4c9d5..79347014562bd 100644 > --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c > +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c [ ... ] > @@ -1105,6 +1106,11 @@ static int e5010_probe(struct platform_device *pdev) > goto fail_after_video_register_device; > } > > + ret = e5010_init_device(e5010); > + if (ret) { > + dev_err_probe(dev, ret, "failed to init device\n"); > + goto fail_after_clock_enable; > + } > > ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0); > if (ret) { > @@ -1112,7 +1118,11 @@ static int e5010_probe(struct platform_device *pdev) > goto fail_after_clock_enable; > } > > + pm_runtime_set_autosuspend_delay(dev, 100); > + pm_runtime_use_autosuspend(dev); > + pm_runtime_set_active(dev); > pm_runtime_enable(dev); > + pm_runtime_idle(dev); [Severity: High] This is a pre-existing issue, but registering the video device before runtime PM is fully configured and enabled exposes the device prematurely. If a userspace process opens the video device and starts streaming immediately after video_register_device() but before pm_runtime_enable(), e5010_device_run() will fail in pm_runtime_resume_and_get(). Due to the new changes in this patch, this failure now triggers an infinite scheduling loop. Should the video device registration be moved after runtime PM is enabled? Also related to the probe function, the e5010 context is allocated with devres: e5010 = devm_kzalloc(dev, sizeof(*e5010), GFP_KERNEL); [Severity: High] This is a pre-existing issue, but does this devres allocation tie the driver context to the device lifetime rather than the video device file operations? If the driver is unbound while a process has the device open, devres will free the e5010 struct immediately after e5010_remove(). When userspace later closes the file descriptor, e5010_release() accesses the freed struct: static int e5010_release(struct file *file) { struct e5010_dev *e5010 = video_drvdata(file); ... mutex_lock(&e5010->mutex); Can this lead to a use-after-free on late file close? [ ... ] > @@ -1353,7 +1343,15 @@ static void e5010_device_run(void *priv) > unsigned long flags; > int num_planes = ctx->out_queue.fmt->num_planes; > > + ret = pm_runtime_resume_and_get(e5010->dev); > + if (ret < 0) { > + dev_err(e5010->dev, "Device failed to turn on\n"); > + v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx); > + return; > + } > + > spin_lock_irqsave(&e5010->hw_lock, flags); [Severity: High] If pm_runtime_resume_and_get() fails here, the function finishes the job but returns without removing the ready source and destination buffers from the queues. Since the context remains in a ready state, won't the V4L2 mem2mem framework immediately reschedule the job, causing an infinite scheduling loop? Should the ready buffers be dequeued and marked as an error before calling v4l2_m2m_job_finish()? [ ... ] For the PM transitions introduced in this patch, I noticed a potential issue in the remove function: static void e5010_remove(struct platform_device *pdev) { struct e5010_dev *e5010 = platform_get_drvdata(pdev); pm_runtime_disable(e5010->dev); clk_disable_unprepare(e5010->clk); [Severity: High] This patch enables autosuspend, meaning the device could be suspended and its clock already disabled when e5010_remove() is called. Since pm_runtime_disable() does not resume a suspended device, won't calling clk_disable_unprepare() unconditionally cause a clock reference count underflow? Does the device need to be resumed first, or should the clock disable be conditioned on the runtime PM state? [Severity: High] This is a pre-existing issue, but also in e5010_remove(), the m2m_dev is freed before the devres-managed IRQ handler is disabled: v4l2_m2m_release(e5010->m2m_dev); If an interrupt fires concurrently and e5010_irq() is executing during unbind, it calls v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx) and accesses the freed m2m_dev. Does the IRQ handler need to be explicitly disabled or freed before releasing the m2m device? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831213231.2125544-1-b-brnich@ti.com?par…

1 0

Re: [PATCH v4 3/4] media: imagination: e5010: Move e5010_init_device to Runtime Resume Hook
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Use-after-free on `struct e5010_dev` after device unbind due to `devm_kzalloc` mismatch with video device lifetime. - [High] Use-after-free of `m2m_dev` in the IRQ handler due to asymmetric teardown. -- commit 3b88e88daceec2c17fb0dd5eee30284b2ce3e570 Author: Brandon Brnich <b-brnich(a)ti.com> media: imagination: e5010: Move e5010_init_device to Runtime Resume Hook The PM framework skips the runtime resume hook if the device was never on in the first place. This results in a crash in the system resume hook when a call to e5010_init_device attempts a write to a powered down device. Move e5010_init_device to the resume hook to ensure register write only occurs when device is properly powered on. > diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c > index e98fcd3fcd9ae..488f5d7e4c9d5 100644 > --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c > +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c [ ... ] > @@ -1511,6 +1511,13 @@ static int e5010_runtime_resume(struct device *dev) > return ret; > } > > + ret = e5010_init_device(e5010); > + if (ret) { > + dev_err(dev, "Failed to re-enable e5010 device\n"); > + clk_disable_unprepare(e5010->clk); > + return ret; > + } > + > return 0; > } [Severity: High] This is a pre-existing issue, but does e5010_remove() free e5010->m2m_dev while the IRQ handler could still access it? In e5010_remove(), v4l2_m2m_release() is called to free the m2m_dev structure before devres cleans up the IRQ handler requested via devm_request_irq(): drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_remove() { ... pm_runtime_disable(e5010->dev); clk_disable_unprepare(e5010->clk); video_unregister_device(e5010->vdev); v4l2_m2m_release(e5010->m2m_dev); v4l2_device_unregister(&e5010->v4l2_dev); } If an interrupt fires during this window, e5010_irq() will dereference the freed e5010->m2m_dev when calling v4l2_m2m_get_curr_priv(): drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_irq() { ... if (!pic_done && !out_addr_err) { spin_unlock(&e5010->hw_lock); return IRQ_NONE; } ctx = v4l2_m2m_get_curr_priv(e5010->m2m_dev); ... } Can this lead to a use-after-free panic if the device is unbound while hardware is active or a spurious interrupt arrives? [Severity: High] This isn't a bug introduced by this patch, but does the devm_kzalloc() allocation of struct e5010_dev mismatch the video device's lifetime, leading to a use-after-free in e5010_release()? In e5010_probe(), the driver structure is allocated with devres: drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_probe() { ... e5010 = devm_kzalloc(dev, sizeof(*e5010), GFP_KERNEL); ... } If a user holds open /dev/videoX and the driver is unbound via sysfs, e5010_remove() unregisters the video device and completes, allowing devres to free e5010_dev. When the user later closes the file descriptor, e5010_release() is called and dereferences the freed e5010 pointer: drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_release() { struct e5010_dev *e5010 = video_drvdata(file); struct e5010_context *ctx = to_e5010_context(file); dprintk(e5010, 1, "Releasing instance: 0x%p, m2m_ctx: 0x%p\n", ctx, ctx->fh.m2m_ctx); mutex_lock(&e5010->mutex); ... } Could this result in memory corruption when the mutex is locked? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831213231.2125544-1-b-brnich@ti.com?par…

1 0

Re: [PATCH v4 2/4] media: imagination: e5010: Fix clk never enabled without CONFIG_PM
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [High] Exposing the video device to userspace before enabling PM runtime creates a race condition that causes early ioctls to fail. - [High] Hardware clock is disabled before the V4L2 device is unregistered, allowing concurrent hardware access that causes kernel panics. Pre-existing issues: - [High] This problem wasn't introduced by this patch, but the V4L2 M2M device is released while file descriptors may still be active, causing a Use-After-Free. - [High] This problem wasn't introduced by this patch, but the hardware maximum output size calculation fails to account for HEADER_SIZE, leading to DMA out-of-bounds writes. -- commit 7d9f3a17fe86713cd9c4813e9190652a322dd6e5 Author: Brandon Brnich <b-brnich(a)ti.com> media: imagination: e5010: Fix clk never enabled without CONFIG_PM clk_prepare_enable is not called anywhere unless CONFIG_PM is set. This means without CONFIG_PM, the JPEG Encoder will not function properly. Add this call during the probe sequence to properly configure the device. If video_registration fails after clk is enabled, clk would be left on. Make a label to cover this case. > diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c > index d3c11337c815d..e98fcd3fcd9ae 100644 > --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c > +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c > @@ -1099,19 +1099,28 @@ static int e5010_probe(struct platform_device *pdev) [ ... ] > ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0); > if (ret) { > dev_err_probe(dev, ret, "failed to register video device\n"); > - goto fail_after_video_register_device; > + goto fail_after_clock_enable; > } > > + pm_runtime_enable(dev); [Severity: High] Does moving pm_runtime_enable() to after video_register_device() create a race condition? If a userspace application (like udev) opens the device and starts streaming before the probe thread executes pm_runtime_enable(), early ioctls might fail with -EACCES when pm_runtime_resume_and_get() is called. This can cause userspace initialization scripts to fail device probing, rendering the device unusable until manually reset. > + > v4l2_info(&e5010->v4l2_dev, "Device registered as /dev/video%d\n", > e5010->vdev->num); [ ... ] > @@ -1126,6 +1135,7 @@ static void e5010_remove(struct platform_device *pdev) > struct e5010_dev *e5010 = platform_get_drvdata(pdev); > > pm_runtime_disable(e5010->dev); > + clk_disable_unprepare(e5010->clk); [Severity: High] Does disabling the clock here allow concurrent hardware access that could cause a kernel panic? video_unregister_device() prevents new opens, but it hasn't been called yet and it doesn't terminate ongoing ioctls. An active userspace V4L2 operation (e.g., executing e5010_device_run()) might attempt to access hardware registers while the clock is powered off. pm_runtime_disable() does not wait for active usage references to drop to zero, so concurrent access is not blocked. > video_unregister_device(e5010->vdev); > v4l2_m2m_release(e5010->m2m_dev); [Severity: High] This is a pre-existing issue, but does calling v4l2_m2m_release() here cause a use-after-free? V4L2 does not forcefully close existing file descriptors when a device is unregistered, meaning userspace can continue holding the file descriptor open and issuing ioctls. When userspace eventually calls close(), e5010_release() is called, which executes v4l2_m2m_ctx_release(ctx->fh.m2m_ctx). This will access the already-freed m2m_dev and e5010 structures. > v4l2_device_unregister(&e5010->v4l2_dev); [Severity: High] This is a pre-existing issue, but does the hardware maximum output size calculation in e5010_device_run() fail to account for HEADER_SIZE, leading to DMA out-of-bounds writes? In e5010_device_run(), the hardware output base address is advanced: drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_device_run() { ... reg += HEADER_SIZE; ret = e5010_hw_set_output_base_addr(e5010->core_base, reg); ... } However, the hardware maximum output size is incorrectly set to the full buffer length without reducing it by HEADER_SIZE: drivers/media/platform/imagination/e5010-jpeg-enc.c:e5010_device_run() { ... e5010_hw_set_output_max_size(e5010->core_base, d_vb->planes[0].length); ... } This instructs the hardware that it is safe to write length bytes starting from base_addr + HEADER_SIZE. Can this cause the hardware DMA engine to overshoot the destination buffer by exactly HEADER_SIZE bytes? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831213231.2125544-1-b-brnich@ti.com?par…

1 0

Re: [PATCH v4 1/4] media: imagination: e5010: Properly Release m2m_dev if probe fails
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: Pre-existing issues: - [High] The main driver structure and `m2m_dev` are freed early, causing a Use-After-Free when userspace closes open file descriptors. - [High] Potential Use-After-Free of e5010->m2m_dev in e5010_irq() due to devm_request_irq teardown ordering. - [Medium] The interrupt handler accesses device registers without ensuring the device is active via pm_runtime_get_if_active(), risking bus faults. - [Low] Unconditional assignment of PM callbacks causes dead code when PM is disabled. -- commit 94b745ac022d33a06ffc2d161298bb02ee87d0ec Author: Brandon Brnich <b-brnich(a)ti.com> media: imagination: e5010: Properly Release m2m_dev if probe fails After the call to v4l2_m2m_init() is successful, the following initialization functions should go to fail_after_video_register_device. > diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c > index 42ad9ee3993b4..d3c11337c815d 100644 > --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c > +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c > @@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev) > if (IS_ERR(e5010->core_base)) { > ret = PTR_ERR(e5010->core_base); > dev_err_probe(dev, ret, "Missing 'core' resources area\n"); > - goto fail_after_v4l2_register; > + goto fail_after_video_register_device; > } > > e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu"); > if (IS_ERR(e5010->mmu_base)) { > ret = PTR_ERR(e5010->mmu_base); > dev_err_probe(dev, ret, "Missing 'mmu' resources area\n"); > - goto fail_after_v4l2_register; > + goto fail_after_video_register_device; > } [Severity: High] This isn't a bug introduced by this patch, but I noticed a potential use-after-free issue with the m2m_dev when userspace closes open file descriptors. In e5010_probe(), the e5010 structure is allocated with devm_kzalloc(), tying its lifetime strictly to the device bind cycle: e5010 = devm_kzalloc(dev, sizeof(*e5010), GFP_KERNEL); When the device is unbound, e5010_remove() completes and devres frees the structure. It also explicitly frees the m2m device: v4l2_m2m_release(e5010->m2m_dev); However, open V4L2 file descriptors keep the video device alive. When userspace subsequently closes the file descriptor, V4L2 invokes e5010_release(), which dereferences the freed e5010 pointer and the freed m2m device context: v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); Is there a way to decouple the driver structure and m2m device lifetimes from the device bind cycle? [Severity: High] This is a pre-existing issue, but there might be a use-after-free involving e5010->m2m_dev during device teardown due to devm_request_irq() ordering. The interrupt is requested via devm_request_irq() in e5010_probe(). In e5010_remove(), e5010->m2m_dev is explicitly freed: v4l2_m2m_release(e5010->m2m_dev); Because the IRQ is devm-managed, it is not disabled or freed until after e5010_remove() returns. If an interrupt fires during this window, e5010_irq() will dereference the explicitly freed pointer: ctx = v4l2_m2m_get_curr_priv(e5010->m2m_dev); Could the interrupt be manually disabled in e5010_remove() before the m2m device is released? [Severity: Medium] This is another pre-existing issue, but I noticed the interrupt handler might access device registers without ensuring the device is actively powered. If a spurious interrupt fires while the device is in a runtime-suspended state, e5010_irq() unconditionally executes e5010_hw_pic_done_irq(), which reads from MMIO: pic_done = e5010_hw_pic_done_irq(e5010->core_base); Reading registers from unclocked or unpowered hardware can cause a synchronous external abort or return invalid data. Should this interrupt handler use pm_runtime_get_if_active() before accessing hardware registers? [Severity: Low] This isn't a bug introduced by this patch, but it looks like the PM callbacks are assigned unconditionally: static struct platform_driver e5010_driver = { ... .driver = { ... .pm = &e5010_pm_ops, }, }; This prevents the compiler from optimizing out the PM functions when CONFIG_PM is disabled, resulting in dead code. Would it be better to wrap this assignment with the pm_ptr() macro? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831213231.2125544-1-b-brnich@ti.com?par…

1 0

Re: [PATCH v5 16/16] media: i2c: Add WV517S lens actuator driver
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Use-after-free of v4l2_subdev when device file is closed after unbind. - [High] Driver initialization fails completely if CONFIG_PM is disabled due to unhandled -EINVAL from pm_runtime_get_if_active(). -- commit 8edae9366d2e628b71319d675a7b88140fc8e145 Author: Maurizio Casciano <mauriziocasciano7(a)gmail.com> media: i2c: Add WV517S lens actuator driver This commit adds a V4L2 lens subdevice exposing the standard 10-bit FOCUS_ABSOLUTE control and the device ringing-control mode for the WV517S voice-coil actuator. It utilizes regmap for register access and ties control updates to runtime PM. Link: https://github.com/jekhor/yogabook-linux-android-kernel/blob/574bae692716f1… > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 5c52007f9cbeb..c5636e0cbecf5 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig [ ... ] > @@ -949,6 +949,17 @@ config VIDEO_DW9807_VCM > capability. This is designed for linear control of > voice coil motors, controlled via I2C serial interface. > > +config VIDEO_WV517S > + tristate "WV517S lens voice coil support" > + select REGMAP_I2C > + help > + This is a driver for the WV517S camera lens voice coil. The driver [ ... ] > diff --git a/drivers/media/i2c/wv517s.c b/drivers/media/i2c/wv517s.c > new file mode 100644 > index 0000000000000..fa6c6d7524fb7 > --- /dev/null > +++ b/drivers/media/i2c/wv517s.c [ ... ] > +static int wv517s_set_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct wv517s_device *wv517s = > + container_of(ctrl->handler, struct wv517s_device, ctrl_handler); > + struct device *dev = wv517s->sd.dev; > + int pm_ret; > + int ret; > + > + if (ctrl->id != V4L2_CID_FOCUS_ABSOLUTE) > + return -EINVAL; > + > + /* Runtime resume restores controls while the PM state is RPM_RESUMING. */ > + pm_ret = pm_runtime_get_if_active(dev); > + if (!pm_ret && !wv517s->resuming) > + return 0; > + if (pm_ret < 0) > + return pm_ret; [Severity: High] Will this fail driver initialization entirely if CONFIG_PM is disabled? The driver Kconfig does not depend on CONFIG_PM, but without it, pm_runtime_get_if_active() returns -EINVAL. Because wv517s_set_ctrl() treats a negative return as a fatal error, it appears that standard probe sequences updating controls will fail outright when PM support is missing. > + > + ret = regmap_write(wv517s->regmap, WV517S_REG_FOCUS, ctrl->val); > + > + if (pm_ret > 0) > + pm_runtime_put(dev); > + > + return ret; > +} > + > +static const struct v4l2_ctrl_ops wv517s_ctrl_ops = { > + .s_ctrl = wv517s_set_ctrl, > +}; > + > +static int wv517s_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > +{ > + return pm_runtime_resume_and_get(sd->dev); > +} > + > +static int wv517s_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > +{ > + pm_runtime_put(sd->dev); [Severity: Critical] Could this lead to a use-after-free when a file descriptor is closed after the device is unbound? Looking at wv517s_probe(), the subdevice is allocated using devres: wv517s_probe() { ... wv517s = devm_kzalloc(dev, sizeof(*wv517s), GFP_KERNEL); ... } This means it will be freed immediately when the device is unbound. However, if a user space process holds the device node open and closes it after the unbind, wv517s_close() will be called on a freed subdevice, leading to a dereference of the freed sd->dev pointer. Should the allocation be decoupled from devres to match the file descriptor lifecycle? > + > + return 0; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831180101.3109854-1-mauriziocasciano7@g…

1 0

Re: [PATCH v5 15/16] media: atomisp: allow raw Bayer capture
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] `atomisp_enum_fmt_cap` fails to enumerate all supported RAW formats by incorrectly filtering against only the first media-bus code. - [High] RAW formats are incorrectly restricted to standard YUV sizes and padding checks in `atomisp_enum_framesizes_crop_inner`, breaking RAW format enumeration on crop-supporting sensors. - [High] `atomisp_try_fmt` does not recompute padding if a requested RAW format falls back to a processed (e.g., YUV) format, leaking sensor padding pixels into userspace dimensions. -- commit c44ac14c6a4b303046af4f4b358e1df196313c5f Author: Maurizio Casciano <mauriziocasciano7(a)gmail.com> media: atomisp: allow raw Bayer capture This commit enables raw Bayer capture in AtomISP by allowing raw formats to be enumerated and requested. It sets zero padding for raw formats and leaves the receiver crop pipeline configuration to userspace. > diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c > index 759effdb2e8d9..99d2b73b160d7 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c > +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c [ ... ] > @@ -3838,7 +3837,10 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, > * resolution + padding. Add padding here and remove it again after > * the set_fmt call, like atomisp_set_fmt_to_snr() does. > */ > - atomisp_get_pix_padding(isp, f, &padding); > + if (fmt->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) > + padding = (struct v4l2_area) { }; > + else > + atomisp_get_pix_padding(isp, f, &padding); > v4l2_fill_mbus_format(&ffmt, f, fmt->mbus_code); > ffmt.width += padding.width; > ffmt.height += padding.height; > @@ -3858,6 +3860,12 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, > return -EINVAL; > } > > + if (fmt->sh_fmt == IA_CSS_FRAME_FORMAT_RAW && > + fmt->mbus_code != snr_fmt->mbus_code) { > + fmt = snr_fmt; > + f->pixelformat = fmt->pixelformat; > + } > + [Severity: High] Does this code expose hardware padding pixels to userspace if the sensor backend overrides a requested RAW format to a processed (YUV) format? When a RAW format is initially requested, padding is initialized to {0, 0}. If the sensor then rejects the RAW format and falls back to a processed format in snr_fmt, fmt is overwritten. However, padding is not recalculated using atomisp_get_pix_padding() after the fallback. This means the exposed f->width and f->height might incorrectly include internal ISP padding. > f->width = ffmt.width - padding.width; > f->height = ffmt.height - padding.height; > diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c > index 87c29a940f9b2..aa6d13a4de891 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c > +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c > @@ -537,12 +537,21 @@ static int atomisp_enum_framesizes(struct file *file, void *priv, > .which = V4L2_SUBDEV_FORMAT_ACTIVE, > .code = input->code, > }; > + const struct atomisp_format_bridge *format; > struct v4l2_subdev_state *act_sd_state; > + struct v4l2_area padding = { > + .width = pad_w, > + .height = pad_h, > + }; > int ret; > > if (!input->sensor) > return -EINVAL; > > + format = atomisp_get_format_bridge(fsize->pixel_format); > + if (!format) > + return -EINVAL; > + > if (input->crop_support) > return atomisp_enum_framesizes_crop(isp, fsize); [Severity: High] Will VIDIOC_ENUM_FRAMESIZES return bogus values for RAW formats if the sensor supports cropping? By returning early and delegating to atomisp_enum_framesizes_crop() when input->crop_support is true, this path skips the newly added RAW format handling. The delegated atomisp_enum_framesizes_crop_inner() function unconditionally applies ISP padding checks and standard YUV framesize limits, which could break enumeration for RAW formats. [ ... ] > @@ -630,7 +642,8 @@ static int atomisp_enum_fmt_cap(struct file *file, void *fh, > * > * FIXME: fix the pipeline to allow sensor format too. > */ > - if (format->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) > + if (format->sh_fmt == IA_CSS_FRAME_FORMAT_RAW && > + format->mbus_code != code.code) > continue; > > /* Found a match. Now let's pick f->index'th one. */ [Severity: High] Does this code inadvertently hide valid RAW formats from userspace when the sensor supports multiple media-bus codes? The code variable is initialized earlier in atomisp_enum_fmt_cap() with only .which = V4L2_SUBDEV_FORMAT_ACTIVE, leaving .index defaulting to 0. As a result, the v4l2_subdev_call() to enum_mbus_code only retrieves the first sensor format. If this loop filters out any RAW format where mbus_code doesn't exactly match this 0-th index sensor code, it will silently discard other valid RAW formats the sensor might expose (e.g., 8-bit vs 10-bit), breaking the VIDIOC_ENUM_FMT API contract. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831180101.3109854-1-mauriziocasciano7@g…

1 0

Re: [PATCH v5 10/16] media: atomisp: derive CSI-2 timing from sensor link frequency
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The patch introduces an implicit 64-bit integer division on 32-bit architectures, which will cause a link-time build failure (`undefined reference to __divdi3`). -- commit ae3982832f5b3aa722aed79005b9309db80ad812 Author: Maurizio Casciano <mauriziocasciano7(a)gmail.com> media: atomisp: derive CSI-2 timing from sensor link frequency This commit updates the CSI-2 receiver timing configuration to query the sensor link frequency via v4l2_get_link_freq(). It changes the link frequency type to s64 to accommodate valid ranges without an arbitrary limit. > diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c b/drivers/staging/media/atomisp/pci/atomisp_csi2.c > --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c > +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c > @@ -209,7 +210,7 @@ int atomisp_mipi_csi2_register_entities(struct atomisp_mipi_csi2_device *csi2, > static const int LIMIT_SHIFT = 6; /* Limit numeric range into 31 bits */ > > static int > -atomisp_csi2_configure_calc(const short int coeffs[2], int mipi_freq, int def) > +atomisp_csi2_configure_calc(const short int coeffs[2], s64 mipi_freq, int def) > { [Severity: Low] Will this cause a link-time build failure on 32-bit architectures? Since mipi_freq is now an s64, the later calculation in atomisp_csi2_configure_calc performs an implicit 64-bit integer division: r = accinv * coeffs[1] * (500000000 >> LIMIT_SHIFT); r /= mipi_freq >> LIMIT_SHIFT; r += accinv * coeffs[0]; Because the shift operation on an s64 retains the s64 type, the division assigns an int to be divided by an s64. Will this result in an undefined reference to __divdi3 when compiled on architectures like x86-32? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831180101.3109854-1-mauriziocasciano7@g…

1 0

Re: [PATCH v4 5/6] media: ipu-bridge: Match sensor configs per IPU and add config flags
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] The `primary->secondary` fwnode pointer of the ACPI device is left dangling if initialization fails, leading to a potential use-after-free. -- commit 0f441a3f61c5b2b58f9a2384a8ebd71e6f71b364 Author: Fernando Rimoli <fernandorimoli11(a)gmail.com> media: ipu-bridge: Match sensor configs per IPU and add config flags This commit adds an optional IPU PCI product ID and flags to the sensor configuration struct to allow sensors to be treated differently based on the connected IPU. It introduces matching logic that gives precedence to IPU-specific configurations over generic ones. > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index cd3c36d4434fe..38ad3e54e716d 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c [ ... ] > @@ -862,6 +889,9 @@ static int ipu_bridge_connect_sensors(struct ipu_bridge *bridge) > const struct ipu_sensor_config *cfg = > &ipu_supported_sensors[i]; > > + if (!ipu_bridge_config_matches(cfg, bridge)) > + continue; > + > ret = ipu_bridge_connect_sensor(cfg, bridge); > if (ret) > goto err_unregister_sensors; [Severity: High] This is a pre-existing issue, but does this error path leave a dangling pointer on the persistent ACPI device? During initialization, ipu_bridge_connect_sensor() assigns a secondary fwnode to the ACPI device: ipu_bridge_connect_sensor() { ... primary = acpi_fwnode_handle(adev); primary->secondary = fwnode; ... } If a subsequent sensor fails to connect here, or if the IVSC instantiation fails in ipu_bridge_connect_sensor(), the error path jumps to err_unregister_sensors, which calls ipu_bridge_unregister_sensors(). That cleanup function destroys the software nodes and drops the device references: ipu_bridge_unregister_sensors() { ... for (i = 0; i < bridge->n_sensors; i++) { sensor = &bridge->sensors[i]; software_node_unregister_node_group(sensor->group); acpi_dev_put(sensor->adev); ... } } Since the primary->secondary pointer is never cleared from the ACPI device before the struct ipu_bridge instance is freed, could this lead to a use-after-free if the persistent ACPI device is later queried for device properties by a user? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831181858.325109-1-fernandorimoli11@gma…

1 0

Re: [PATCH v5 09/16] media: intel: ipu-bridge: allow sensor-specific link frequencies
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] A work item is queued to `system_long_wq` but never explicitly canceled, risking a Use-After-Free if the module is unloaded. - [High] The error paths in sensor initialization fail to clear secondary fwnode pointers, leading to Use-After-Free when the `bridge` structure is freed. -- commit b4da19e9e36d2d8812f8fa983ad7cffe406ddac1 Author: Maurizio Casciano <mauriziocasciano7(a)gmail.com> media: intel: ipu-bridge: allow sensor-specific link frequencies A bridge-specific firmware parser may have more accurate link-frequency information than the generic ACPI hardware-ID table. Let it store link frequencies in the per-sensor bridge data and prefer those values when constructing the endpoint software node. > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index 47317c423fad5..5730a95767a2f 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c [ ... ] drivers/media/pci/intel/ipu-bridge.c:ipu_bridge_instantiate_vcm() { ... queue_work(system_long_wq, &data->work); return 0; } [Severity: High] This is a pre-existing issue, but the work item is queued to system_long_wq here and never explicitly canceled. If the ipu-bridge module is unloaded before or during its execution, could the work function pointer point to unmapped memory? Does this cause a kernel panic and leak the work structure when system_long_wq executes it? drivers/media/pci/intel/ipu-bridge.c:ipu_bridge_connect_sensor() { ... sensor->adev = ACPI_PTR(acpi_dev_get(adev)); primary = acpi_fwnode_handle(adev); primary->secondary = fwnode; ret = ipu_bridge_instantiate_ivsc(sensor); if (ret) goto err_free_swnodes; ... err_free_swnodes: software_node_unregister_node_group(sensor->group); err_put_ivsc: put_device(sensor->csi_dev); acpi_dev_put(sensor->ivsc_adev); err_put_adev: acpi_dev_put(adev); return ret; } [Severity: High] This isn't a bug introduced by this patch, but the error paths in sensor initialization appear to fail to clear secondary fwnode pointers. If ipu_bridge_instantiate_ivsc() fails, the error path goes to err_free_swnodes but does not clear primary->secondary. It also appears to leak the ACPI device reference taken for sensor->adev. A similar pattern is present in ipu_bridge_unregister_sensors(), which also fails to clear the secondary fwnode pointers. When the bridge structure is later freed during driver probe teardown, does this leave the ACPI device with a dangling pointer and risk a use-after-free panic? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831180101.3109854-1-mauriziocasciano7@g…

1 0

Re: [PATCH v4 4/6] media: ipu-bridge: Assign endpoint property indices dynamically
by sashiko-bot＠kernel.org 31 Aug '26

31 Aug '26

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The bounds-checking macro IPU_NEXT_EP_PROPERTY contains an off-by-one error and incorrectly uses the NULL terminator slot as a fallback index, allowing silent overwrites of the mandatory property array terminator. -- commit db66f5f5a5507fa203b41289d17618deee87aedc Author: Fernando Rimoli <fernandorimoli11(a)gmail.com> media: ipu-bridge: Assign endpoint property indices dynamically This commit dynamically assigns indices to the endpoint properties array to avoid skipping properties if earlier optional slots are left empty. It sizes the array with an enum and assigns indices via a bounds-checked running index. > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index 131c70844052f..cd3c36d4434fe 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c [ ... ] > @@ -508,21 +509,21 @@ static void ipu_bridge_create_fwnode_properties( > PROPERTY_ENTRY_REF_ARRAY("lens-focus", sensor->vcm_ref); > } > > - sensor->ep_properties[0] = PROPERTY_ENTRY_U32( > - sensor->prop_names.bus_type, > - V4L2_FWNODE_BUS_TYPE_CSI2_DPHY); > - sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN( > - sensor->prop_names.data_lanes, > - bridge->data_lanes, sensor->lanes); > - sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY( > - sensor->prop_names.remote_endpoint, > - sensor->local_ref); > + sensor->ep_properties[IPU_NEXT_EP_PROPERTY(i, NUM_OF)] = > + PROPERTY_ENTRY_U32(names->bus_type, > + V4L2_FWNODE_BUS_TYPE_CSI2_DPHY); [ ... ] > diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h > index 16fac765456e8..61e10cef16a31 100644 > --- a/include/media/ipu-bridge.h > +++ b/include/media/ipu-bridge.h [ ... ] > @@ -64,6 +64,23 @@ enum ipu_sensor_swnodes { > SWNODE_COUNT > }; > > +enum ipu_sensor_ep_props { > + IPU_SENSOR_EP_BUS_TYPE, > + IPU_SENSOR_EP_DATA_LANES, > + IPU_SENSOR_EP_REMOTE_EP, > + IPU_SENSOR_EP_LINK_FREQUENCIES, > + IPU_SENSOR_EP_NUM_OF, > + IPU_SENSOR_EP_NUM_ENTRIES > +}; > + > +/* > + * Get the index of the next endpoint property in the property array, with a > + * given maximum value. > + */ > +#define IPU_NEXT_EP_PROPERTY(index, max) \ > + (WARN_ON((index) > IPU_SENSOR_EP_##max) ? \ > + IPU_SENSOR_EP_##max : (index)++) [Severity: Medium] Could this bounds-checking macro allow a silent overwrite of the mandatory property array terminator? It looks like the condition uses > instead of >=. Because the macro is called with NUM_OF in ipu_bridge_create_fwnode_properties() as shown above, if the index equals the terminator slot (IPU_SENSOR_EP_NUM_OF), the check evaluates to false and silently allows the write into the terminator slot without a warning. Also, when index is greater than max, the fallback value is IPU_SENSOR_EP_##max. Does this fallback explicitly direct the out-of-bounds write exactly into the terminator slot? Looking at the layout of struct ipu_sensor: struct ipu_sensor { ... struct property_entry ep_properties[IPU_SENSOR_EP_NUM_ENTRIES]; struct property_entry dev_properties[5]; ... } Because dev_properties immediately follows ep_properties, overwriting the NULL terminator in the ep_properties array could cause fwnode parsers like property_entries_dup() to read past the end of the array into the adjacent dev_properties memory, parsing invalid properties. While there are currently exactly enough macro calls to avoid reaching NUM_OF, any future addition might trigger this issue. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260831181858.325109-1-fernandorimoli11@gma…

1 0

2026

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

media-ci August 2026