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

Pre-existing issues:
- [High] Kernel panic due to division by zero when calculating audio input 
frequency.
- [High] Kernel panic due to division by zero when calculating video pixel 
clock.
--

--- Patch [3]: [PATCH v4 3/5] drm/bridge: it6505: guard against zero channel 
count in audio infoframe ---
commit 8ace85a52fcbf9bce4f2a154064b6799263b545d
Author: Daniel Golle <[email protected]>
Subject: drm/bridge: it6505: guard against zero channel count in audio infoframe

This commit guards against a zero channel count in 
it6505_enable_audio_infoframe()
to prevent an out-of-bounds stack read. It also fixes a debug print logging an
incorrect channel count on failure.

> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
> b/drivers/gpu/drm/bridge/ite-it6505.c
> index 2a8298ee6c03c..443ea55797441 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c

[ ... ]

While reviewing this, I noticed a couple of pre-existing issues in this file:

drivers/gpu/drm/bridge/ite-it6505.c:it6505_calc_video_info() {
        ...
        if (sum == 0) {
                DRM_DEV_DEBUG_DRIVER(dev, "calc video timing error");
                return;
        }

        sum /= 3;
        pclk = 13500 * 2048 / sum;
        ...
}

[Severity: High]
This is a pre-existing issue, but can this integer division shrink small sums
to 0?

If the accumulated readings are very small (e.g., sum is 1 or 2), the
integer division sum /= 3 will truncate the sum to 0.

The subsequent division for pclk will divide by zero and panic the kernel.

Should there be a check to ensure sum is still non-zero after the division
by 3?

[ ... ]

> @@ -1575,6 +1575,9 @@ static void it6505_enable_audio_infoframe(struct it6505 
> *it6505)
>       struct device *dev = it6505->dev;
>       u8 audio_info_ca[] = { 0x00, 0x00, 0x01, 0x03, 0x07, 0x0B, 0x0F, 0x1F };
>  
> +     if (!it6505->audio.channel_count)
> +             return;
> +
>       DRM_DEV_DEBUG_DRIVER(dev, "infoframe channel_allocation:0x%02x",
>                            audio_info_ca[it6505->audio.channel_count - 1]);
>  

drivers/gpu/drm/bridge/ite-it6505.c:it6505_enable_audio() {
        ...
        regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
        DRM_DEV_DEBUG_DRIVER(dev, "regbe:0x%02x audio input fs: %d.%d kHz",
                             regbe, 6750 / regbe, (6750 % regbe) * 10 / regbe);
        ...
}

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for regbe to
be 0 here?

If the hardware measures no audio clock, this register could return 0.

The debug print macro then divides by regbe, which would cause a kernel
panic due to division by zero.

Could we check if regbe is non-zero before using it as a divisor?

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to