lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 8 Jul 2019 08:02:55 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Rex Zhu <rex.zhu@....com>, Evan Quan <evan.quan@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Christian K├Ânig <christian.koenig@....com>,
        "David (ChunMing) Zhou" <David1.Zhou@....com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Chengming Gui <Jack.Gui@....com>,
        Kevin Wang <kevin1.wang@....com>, linux-kernel@...r.kernel.org,
        amd-gfx@...ts.freedesktop.org, Huang Rui <ray.huang@....com>,
        dri-devel@...ts.freedesktop.org, Likun Gao <Likun.Gao@....com>,
        Hawking Zhang <Hawking.Zhang@....com>
Subject: Re: [1/2] drm/amd/powerplay: smu_v11_0: fix uninitialized variable
 use

On Mon, Jul 08, 2019 at 04:07:58PM +0200, Arnd Bergmann wrote:
> A mistake in the error handling caused an uninitialized
> variable to be used:
> 
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: error: variable 'freq' is used uninitialized whenever '?:' condition is false [-Werror,-Wsometimes-uninitialized]
>                 ret =  smu_get_current_clk_freq_by_table(smu, clk_id, &freq);
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note: expanded from macro 'smu_get_current_clk_freq_by_table'
>         ((smu)->ppt_funcs->get_current_clk_freq_by_table ? (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) : 0)
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1114:2: note: uninitialized use occurs here
>         freq *= 100;
>         ^~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: note: remove the '?:' if its condition is always true
>                 ret =  smu_get_current_clk_freq_by_table(smu, clk_id, &freq);
>                        ^
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note: expanded from macro 'smu_get_current_clk_freq_by_table'
>         ((smu)->ppt_funcs->get_current_clk_freq_by_table ? (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) : 0)
>          ^
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1095:15: note: initialize the variable 'freq' to silence this warning
>         uint32_t freq;
>                      ^
>                       = 0
> 
> Bail out of smu_v11_0_get_current_clk_freq() before we get there.
> 
> Fixes: e36182490dec ("drm/amd/powerplay: fix dpm freq unit error (10KHz -> Mhz)")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index c3f9714e9047..bd89a13b6679 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1099,9 +1099,11 @@ static int smu_v11_0_get_current_clk_freq(struct smu_context *smu,
>  		return -EINVAL;
>  
>  	/* if don't has GetDpmClockFreq Message, try get current clock by SmuMetrics_t */
> -	if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0)
> +	if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) {
>  		ret =  smu_get_current_clk_freq_by_table(smu, clk_id, &freq);
> -	else {
> +		if (ret)
> +			return ret;

I am kind of surprised that this fixes the warning. If I am interpreting
the warning correctly, it is complaining that the member
get_current_clk_freq_by_table could be NULL and not be called to
initialize freq and that entire statement will become 0. If that is the
case, it seems like this added error handling won't fix that problem,
right?

Cheers,
Nathan

Powered by blists - more mailing lists