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

Pre-existing issues:
- [High] NULL pointer dereference when logging errors in 
`dce112_get_pix_clk_dividers` and `dcn3_get_pix_clk_dividers`.
--

--- Patch [1]: [PATCH] drm/amd/display: use proper context for logging ---
commit 67013b697b017867bb1c1e61f13b863b96364c5e
Author: Jiri Slaby (SUSE) <[email protected]>

drm/amd/display: use proper context for logging

This commit fixes a NULL pointer dereference in get_ss_info_from_atombios()
by temporarily redefining DC_LOGGER to use clk_src->base.ctx->logger. The
calc_pll_cs->ctx context is uninitialized at that time since it is only set
later by calc_pll_max_vco_construct().

Closes: https://bugzilla.suse.com/show_bug.cgi?id=1271175
Closes: 
https://lore.kernel.org/all/[email protected]/
Fixes: 1296423bf23c ("drm/amd/display: define DC_LOGGER for logger")

> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c 
> b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> index ecb8493ec5233..d3f7aa853e3a6 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> @@ -1513,6 +1513,10 @@ static const struct clock_source_funcs 
> dce110_clk_src_funcs = {
>       .get_dp_dto_frequency_100hz = get_dp_dto_frequency_100hz
>  };
>  
> +/* calc_pll_cs->ctx is set only after get_ss_info_from_atombios(), unlike 
> clk_src->base */
> +#pragma push_macro("DC_LOGGER")
> +#undef DC_LOGGER
> +#define DC_LOGGER CTX->logger

[Severity: High]
This is a pre-existing issue, but does a similar NULL pointer dereference 
exist in dce112_get_pix_clk_dividers() and dcn3_get_pix_clk_dividers()
during their error paths?

It appears dce112_clk_src_construct() does not call 
calc_pll_max_vco_construct(), leaving calc_pll_cs->ctx as NULL. If invalid 
parameters are passed (like requested_pix_clk_100hz == 0), the error path
invokes DC_LOG_ERROR() which dereferences calc_pll_cs->ctx->logger:

drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c:dce112_get_pix_clk_dividers()
 {
        DC_LOGGER_INIT();

        if (pix_clk_params == NULL || pll_settings == NULL
                        || pix_clk_params->requested_pix_clk_100hz == 0) {
                DC_LOG_ERROR(
                        "%s: Invalid parameters!!\n", __func__);
                return (uint32_t)-1;
        }
        ...
}

A similar error path exists in dcn3_get_pix_clk_dividers().

Since DC_LOG_ERROR() relies on the same uninitialized context pointer, 
could this trigger a crash similar to the one being fixed here if these
functions encounter invalid parameters?

[ ... ]

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

Reply via email to