Re: [PATCH v16 11/12] media: mediatek: jpeg: add jpeg compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check on the number of DT child nodes causes out-of-bounds array accesses and stack overflows.
- [High] Incorrect calculation of the 34-bit extension address for the encoder's stall address.
--

--- Patch [11]: [PATCH v16 11/12] media: mediatek: jpeg: add jpeg compatible ---
Note: The format of this report has been altered to provide a free-form
summary without quoting the original patch code, due to recitation
restrictions.

commit f9b0fac0a51678a6983018a9ca39e5cce2cb6604
Author: Kyrie Wu <kyrie.wu@mediatek.com>
media: mediatek: jpeg: add jpeg compatible

This commit adds JPEG encoder and decoder compatibles for the mt8196
platform. It sets up the variant data structures with appropriate
capabilities like multi-core and 34-bit support.

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the new
compatibles, I noticed a potential stack overflow in the core driver.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c:mtk_jpeg_probe(),
the code iterates over the device tree child nodes and sets
jpeg->max_hw_count to the total number of children, without capping it.

Later, in mtk_jpegenc_worker() and mtk_jpegdec_worker(), this unbounded
jpeg->max_hw_count is used as a loop limit to copy elements into the
comp_jpeg array, which is a fixed-size stack array. Additionally, in
mtk_jpegenc_hw_probe(), the hw_index is incremented and used without
bounds checking to store hardware instances into master_dev->enc_hw_dev.

Could a malformed device tree with more child nodes than the hardware limit
cause a stack buffer overflow or out-of-bounds write here? Should we add
bounds checking against MTK_JPEGENC_HW_MAX and MTK_JPEGDEC_HW_MAX?

[Severity: High]
This isn't a bug introduced by this patch, but adding support_34bit = true
for mt8196_jpegenc_drvdata exposes an existing issue in the encoder address
calculation.

In drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c at
mtk_jpeg_set_enc_dst(), when 34-bit support is enabled, the code calculates
the extended address using FIELD_PREP on the upper 32 bits of dma_addr.

It then writes the sum of this extended address and the buffer size to the
JPEG_ENC_STALL_ADDR0_EXT register. Because the extended address is a masked
bitfield representing the upper 32 bits, adding the raw byte length size
directly to this bitfield seems incorrect.

Could this result in writing an invalid, unmasked address extension to the
hardware registers, potentially leading to SMMU faults or memory corruption?
Should this instead calculate the upper 32 bits of the sum of dma_addr
and size?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133432.1042046-1-kyrie.wu@mediatek.com?part=11