| 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 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 6 Aug 2019 10:10:25 -0400
From: Alex Deucher <alexdeucher@...il.com>
To: "Quan, Evan" <Evan.Quan@....com>
Cc: Nathan Chancellor <natechancellor@...il.com>,
"Deucher, Alexander" <Alexander.Deucher@....com>,
"Koenig, Christian" <Christian.Koenig@....com>,
"Zhou, David(ChunMing)" <David1.Zhou@....com>,
"clang-built-linux@...glegroups.com"
<clang-built-linux@...glegroups.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/amd/powerplay: Zero initialize some variables
Applied. Thanks!
Alex
On Sun, Aug 4, 2019 at 9:21 PM Quan, Evan <Evan.Quan@....com> wrote:
>
> Thanks Nathan. The patch is reviewed-by: Evan Quan <evan.quan@....com>
>
> > -----Original Message-----
> > From: Nathan Chancellor <natechancellor@...il.com>
> > Sent: Monday, August 05, 2019 4:37 AM
> > To: Quan, Evan <Evan.Quan@....com>; Deucher, Alexander
> > <Alexander.Deucher@....com>; Koenig, Christian
> > <Christian.Koenig@....com>; Zhou, David(ChunMing)
> > <David1.Zhou@....com>
> > Cc: amd-gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-
> > kernel@...r.kernel.org; clang-built-linux@...glegroups.com; Nathan
> > Chancellor <natechancellor@...il.com>
> > Subject: [PATCH] drm/amd/powerplay: Zero initialize some variables
> >
> > Clang warns (only Navi warning shown but Arcturus warns as well):
> >
> > drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: warning:
> > variable 'asic_default_power_limit' is used uninitialized whenever '?:'
> > condition is false [-Wsometimes-uninitialized]
> > smu_read_smc_arg(smu, &asic_default_power_limit);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3:
> > note:
> > expanded from macro 'smu_read_smc_arg'
> > ((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu),
> > (arg)) : 0)
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1550:30: note:
> > uninitialized use occurs here
> > smu->default_power_limit = asic_default_power_limit;
> > ^~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: note:
> > remove the '?:' if its condition is always true
> > smu_read_smc_arg(smu, &asic_default_power_limit);
> > ^
> > drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3:
> > note:
> > expanded from macro 'smu_read_smc_arg'
> > ((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu),
> > (arg)) : 0)
> > ^
> > drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1517:35: note:
> > initialize the variable 'asic_default_power_limit' to silence this warning
> > uint32_t asic_default_power_limit;
> > ^
> > = 0
> > 1 warning generated.
> >
> > As the code is currently written, if read_smc_arg were ever NULL, arg would
> > fail to be initialized but the code would continue executing as normal
> > because the return value would just be zero.
> >
> > There are a few different possible solutions to resolve this class of warnings
> > which have appeared in these drivers before:
> >
> > 1. Assume the function pointer will never be NULL and eliminate the
> > wrapper macros.
> >
> > 2. Have the wrapper macros initialize arg when the function pointer is
> > NULL.
> >
> > 3. Have the wrapper macros return an error code instead of 0 when the
> > function pointer is NULL so that the callsites can properly bail out
> > before arg can be used.
> >
> > 4. Initialize arg at the top of its function.
> >
> > Number four is the path of least resistance right now as every other change
> > will be driver wide so do that here. I only make the comment now as food for
> > thought.
> >
> > Fixes: b4af964e75c4 ("drm/amd/powerplay: make power limit retrieval as
> > asic specific")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/627
> > Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
> > ---
> > drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 +-
> > drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > index 215f7173fca8..b92eded7374f 100644
> > --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > @@ -1326,7 +1326,7 @@ static int arcturus_get_power_limit(struct
> > smu_context *smu,
> > bool asic_default)
> > {
> > PPTable_t *pptable = smu->smu_table.driver_pptable;
> > - uint32_t asic_default_power_limit;
> > + uint32_t asic_default_power_limit = 0;
> > int ret = 0;
> > int power_src;
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> > index 106352a4fb82..d844bc8411aa 100644
> > --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> > @@ -1514,7 +1514,7 @@ static int navi10_get_power_limit(struct
> > smu_context *smu,
> > bool asic_default)
> > {
> > PPTable_t *pptable = smu->smu_table.driver_pptable;
> > - uint32_t asic_default_power_limit;
> > + uint32_t asic_default_power_limit = 0;
> > int ret = 0;
> > int power_src;
> >
> > --
> > 2.23.0.rc1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Powered by blists - more mailing lists