[PATCH] drm/amd/powerplay: add NULL check for vega10_ps

[PATCH] drm/amd/powerplay: add NULL check for vega10_ps

[Markov Gleb]

From: Gleb Markov <markov.gi@npc-ksb.ru>

cast_phw_vega10_power_state() can return NULL and the return result must
be checked.

Add a NULL check to avoid NULL derederence.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: f83a9991648b ("drm/amd/powerplay: add Vega10 powerplay support (v5)")
Co-developed-by: Mikhail Titov <titov.ma@npc-ksb.ru>
Signed-off-by: Mikhail Titov <titov.ma@npc-ksb.ru>
Signed-off-by: Gleb Markov <markov.gi@npc-ksb.ru>
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index c8c9fb827bda..83e62ab9c7f4 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -3844,6 +3844,8 @@ static uint32_t vega10_dpm_get_sclk(struct pp_hwmgr *hwmgr, bool low)
 		return -EINVAL;
 
 	vega10_ps = cast_phw_vega10_power_state(&ps->hardware);
+	if (vega10_ps == NULL)
+		return -EINVAL;
 
 	if (low)
 		return vega10_ps->performance_levels[0].gfx_clock;
@@ -3866,6 +3868,8 @@ static uint32_t vega10_dpm_get_mclk(struct pp_hwmgr *hwmgr, bool low)
 		return -EINVAL;
 
 	vega10_ps = cast_phw_vega10_power_state(&ps->hardware);
+	if (vega10_ps == NULL)
+		return -EINVAL;
 
 	if (low)
 		return vega10_ps->performance_levels[0].mem_clock;
-- 
2.43.0

Re: [PATCH] drm/amd/powerplay: add NULL check for vega10_ps

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch adds NULL checks for the Vega10 hardware manager but misses identical vulnerabilities in the SMU7 and SMU8 hardware managers.
--

commit 3281774cdb4d49d848f2310e1feaba91d2dc6884
Author: Gleb Markov <markov.gi@npc-ksb.ru>

drm/amd/powerplay: add NULL check for vega10_ps

This patch addresses SVACE static analyzer findings by adding NULL checks for
the return value of cast_phw_vega10_power_state() in the Vega10 hardware
manager to prevent potential NULL pointer dereferences.

> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> index 076e10f265463..c81aa245b8a41 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -3892,6 +3892,8 @@ static uint32_t vega10_dpm_get_sclk(struct pp_hwmgr *hwmgr, bool low)
>  		return -EINVAL;
>  
>  	vega10_ps = cast_phw_vega10_power_state(&ps->hardware);
> +	if (vega10_ps == NULL)
> +		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but while you are addressing this
SVACE warning for Vega10, does the exact same missing NULL check pattern exist 
in the SMU7 and SMU8 hardware managers?

For example, in drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c:smu7_dpm_get_sclk():

	smu7_ps = cast_phw_smu7_power_state(&ps->hardware);

	if (low)
		return smu7_ps->performance_levels[0].engine_clock;

And in drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c:smu8_dpm_get_sclk():

	smu8_ps = cast_smu8_power_state(&ps->hardware);

	if (low)
		return smu8_ps->levels[0].engineClock;

Should identical NULL checks be added to smu7_dpm_get_sclk(),
smu7_dpm_get_mclk(), smu8_dpm_get_sclk(), and smu8_dpm_patch_boot_state()
to ensure a complete fix across the subsystem and avoid kernel panics if
the power state magic number check fails?

>  
>  	if (low)
>  		return vega10_ps->performance_levels[0].gfx_clock;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629130951.763-1-markov.gi@npc-ksb.ru?part=1