* [PATCH] drm/amdgpu: fix potential null dereference
@ 2022-03-02 4:35 Weiguo Li
2022-03-02 14:21 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Weiguo Li @ 2022-03-02 4:35 UTC (permalink / raw)
To: alexander.deucher; +Cc: amd-gfx
"ctx" is dereferenced but null checked later. Swap their positions
to avoid potential null dereference.
Found using a Coccinelle script:
https://coccinelle.gitlabpages.inria.fr/website/rules/mini_null_ref.cocci
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index f522b52725e4..b4f035ce44bc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -258,11 +258,12 @@ static void amdgpu_ctx_fini_entity(struct amdgpu_ctx_entity *entity)
static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
u32 *stable_pstate)
{
- struct amdgpu_device *adev = ctx->adev;
+ struct amdgpu_device *adev;
enum amd_dpm_forced_level current_level;
if (!ctx)
return -EINVAL;
+ adev = ctx->adev;
current_level = amdgpu_dpm_get_performance_level(adev);
@@ -289,12 +290,13 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
u32 stable_pstate)
{
- struct amdgpu_device *adev = ctx->adev;
+ struct amdgpu_device *adev;
enum amd_dpm_forced_level level;
int r;
if (!ctx)
return -EINVAL;
+ adev = ctx->adev;
mutex_lock(&adev->pm.stable_pstate_ctx_lock);
if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: fix potential null dereference
2022-03-02 4:35 [PATCH] drm/amdgpu: fix potential null dereference Weiguo Li
@ 2022-03-02 14:21 ` Alex Deucher
2022-03-02 15:01 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2022-03-02 14:21 UTC (permalink / raw)
To: Weiguo Li; +Cc: Deucher, Alexander, amd-gfx list
Applied. Thanks!
Alex
On Wed, Mar 2, 2022 at 3:56 AM Weiguo Li <liwg06@foxmail.com> wrote:
>
> "ctx" is dereferenced but null checked later. Swap their positions
> to avoid potential null dereference.
>
> Found using a Coccinelle script:
> https://coccinelle.gitlabpages.inria.fr/website/rules/mini_null_ref.cocci
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index f522b52725e4..b4f035ce44bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -258,11 +258,12 @@ static void amdgpu_ctx_fini_entity(struct amdgpu_ctx_entity *entity)
> static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> u32 *stable_pstate)
> {
> - struct amdgpu_device *adev = ctx->adev;
> + struct amdgpu_device *adev;
> enum amd_dpm_forced_level current_level;
>
> if (!ctx)
> return -EINVAL;
> + adev = ctx->adev;
>
> current_level = amdgpu_dpm_get_performance_level(adev);
>
> @@ -289,12 +290,13 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
> u32 stable_pstate)
> {
> - struct amdgpu_device *adev = ctx->adev;
> + struct amdgpu_device *adev;
> enum amd_dpm_forced_level level;
> int r;
>
> if (!ctx)
> return -EINVAL;
> + adev = ctx->adev;
>
> mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: fix potential null dereference
2022-03-02 14:21 ` Alex Deucher
@ 2022-03-02 15:01 ` Christian König
2022-03-02 15:39 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2022-03-02 15:01 UTC (permalink / raw)
To: Alex Deucher, Weiguo Li; +Cc: Deucher, Alexander, amd-gfx list
Looks like my response never made it through.
This change and the existing check is nonsense. The caller makes sure
that ctx is never NULL here.
So the check should probably just be dropped entirely.
Regards,
Christian.
Am 02.03.22 um 15:21 schrieb Alex Deucher:
> Applied. Thanks!
>
> Alex
>
> On Wed, Mar 2, 2022 at 3:56 AM Weiguo Li <liwg06@foxmail.com> wrote:
>> "ctx" is dereferenced but null checked later. Swap their positions
>> to avoid potential null dereference.
>>
>> Found using a Coccinelle script:
>> https://coccinelle.gitlabpages.inria.fr/website/rules/mini_null_ref.cocci
>>
>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index f522b52725e4..b4f035ce44bc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -258,11 +258,12 @@ static void amdgpu_ctx_fini_entity(struct amdgpu_ctx_entity *entity)
>> static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
>> u32 *stable_pstate)
>> {
>> - struct amdgpu_device *adev = ctx->adev;
>> + struct amdgpu_device *adev;
>> enum amd_dpm_forced_level current_level;
>>
>> if (!ctx)
>> return -EINVAL;
>> + adev = ctx->adev;
>>
>> current_level = amdgpu_dpm_get_performance_level(adev);
>>
>> @@ -289,12 +290,13 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
>> static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
>> u32 stable_pstate)
>> {
>> - struct amdgpu_device *adev = ctx->adev;
>> + struct amdgpu_device *adev;
>> enum amd_dpm_forced_level level;
>> int r;
>>
>> if (!ctx)
>> return -EINVAL;
>> + adev = ctx->adev;
>>
>> mutex_lock(&adev->pm.stable_pstate_ctx_lock);
>> if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/amdgpu: fix potential null dereference
2022-03-02 15:01 ` Christian König
@ 2022-03-02 15:39 ` Alex Deucher
2022-03-02 16:17 ` [PATCH v2] drm/amdgpu: remove redundant null check Weiguo Li
0 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2022-03-02 15:39 UTC (permalink / raw)
To: Christian König; +Cc: Weiguo Li, Deucher, Alexander, amd-gfx list
On Wed, Mar 2, 2022 at 10:01 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Looks like my response never made it through.
>
> This change and the existing check is nonsense. The caller makes sure
> that ctx is never NULL here.
>
> So the check should probably just be dropped entirely.
agreed and dropped. Weiguo care to send a patch to drop the checks?
Alex
>
> Regards,
> Christian.
>
> Am 02.03.22 um 15:21 schrieb Alex Deucher:
> > Applied. Thanks!
> >
> > Alex
> >
> > On Wed, Mar 2, 2022 at 3:56 AM Weiguo Li <liwg06@foxmail.com> wrote:
> >> "ctx" is dereferenced but null checked later. Swap their positions
> >> to avoid potential null dereference.
> >>
> >> Found using a Coccinelle script:
> >> https://coccinelle.gitlabpages.inria.fr/website/rules/mini_null_ref.cocci
> >>
> >> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> >> index f522b52725e4..b4f035ce44bc 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> >> @@ -258,11 +258,12 @@ static void amdgpu_ctx_fini_entity(struct amdgpu_ctx_entity *entity)
> >> static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> >> u32 *stable_pstate)
> >> {
> >> - struct amdgpu_device *adev = ctx->adev;
> >> + struct amdgpu_device *adev;
> >> enum amd_dpm_forced_level current_level;
> >>
> >> if (!ctx)
> >> return -EINVAL;
> >> + adev = ctx->adev;
> >>
> >> current_level = amdgpu_dpm_get_performance_level(adev);
> >>
> >> @@ -289,12 +290,13 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> >> static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
> >> u32 stable_pstate)
> >> {
> >> - struct amdgpu_device *adev = ctx->adev;
> >> + struct amdgpu_device *adev;
> >> enum amd_dpm_forced_level level;
> >> int r;
> >>
> >> if (!ctx)
> >> return -EINVAL;
> >> + adev = ctx->adev;
> >>
> >> mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> >> if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] drm/amdgpu: remove redundant null check
2022-03-02 15:39 ` Alex Deucher
@ 2022-03-02 16:17 ` Weiguo Li
2022-03-02 16:53 ` Christian König
0 siblings, 1 reply; 7+ messages in thread
From: Weiguo Li @ 2022-03-02 16:17 UTC (permalink / raw)
To: alexander.deucher; +Cc: ckoenig.leichtzumerken, amd-gfx
Remove the redundant null check since the caller ensures
that 'ctx' is never NULL.
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
---
v2:
* take Christian and Alex's suggestion
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index f522b52725e4..2f38de406937 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -261,9 +261,6 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
struct amdgpu_device *adev = ctx->adev;
enum amd_dpm_forced_level current_level;
- if (!ctx)
- return -EINVAL;
-
current_level = amdgpu_dpm_get_performance_level(adev);
switch (current_level) {
@@ -293,9 +290,6 @@ static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
enum amd_dpm_forced_level level;
int r;
- if (!ctx)
- return -EINVAL;
-
mutex_lock(&adev->pm.stable_pstate_ctx_lock);
if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
r = -EBUSY;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/amdgpu: remove redundant null check
2022-03-02 16:17 ` [PATCH v2] drm/amdgpu: remove redundant null check Weiguo Li
@ 2022-03-02 16:53 ` Christian König
2022-03-02 18:05 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2022-03-02 16:53 UTC (permalink / raw)
To: Weiguo Li, alexander.deucher; +Cc: amd-gfx
Am 02.03.22 um 17:17 schrieb Weiguo Li:
> Remove the redundant null check since the caller ensures
> that 'ctx' is never NULL.
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> v2:
> * take Christian and Alex's suggestion
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index f522b52725e4..2f38de406937 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -261,9 +261,6 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> struct amdgpu_device *adev = ctx->adev;
> enum amd_dpm_forced_level current_level;
>
> - if (!ctx)
> - return -EINVAL;
> -
> current_level = amdgpu_dpm_get_performance_level(adev);
>
> switch (current_level) {
> @@ -293,9 +290,6 @@ static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
> enum amd_dpm_forced_level level;
> int r;
>
> - if (!ctx)
> - return -EINVAL;
> -
> mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
> r = -EBUSY;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/amdgpu: remove redundant null check
2022-03-02 16:53 ` Christian König
@ 2022-03-02 18:05 ` Alex Deucher
0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2022-03-02 18:05 UTC (permalink / raw)
To: Christian König; +Cc: Weiguo Li, Deucher, Alexander, amd-gfx list
Applied. Thanks!
Alex
On Wed, Mar 2, 2022 at 11:54 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 02.03.22 um 17:17 schrieb Weiguo Li:
> > Remove the redundant null check since the caller ensures
> > that 'ctx' is never NULL.
> >
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
> > ---
> > v2:
> > * take Christian and Alex's suggestion
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > index f522b52725e4..2f38de406937 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > @@ -261,9 +261,6 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
> > struct amdgpu_device *adev = ctx->adev;
> > enum amd_dpm_forced_level current_level;
> >
> > - if (!ctx)
> > - return -EINVAL;
> > -
> > current_level = amdgpu_dpm_get_performance_level(adev);
> >
> > switch (current_level) {
> > @@ -293,9 +290,6 @@ static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
> > enum amd_dpm_forced_level level;
> > int r;
> >
> > - if (!ctx)
> > - return -EINVAL;
> > -
> > mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> > if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
> > r = -EBUSY;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-02 18:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-02 4:35 [PATCH] drm/amdgpu: fix potential null dereference Weiguo Li
2022-03-02 14:21 ` Alex Deucher
2022-03-02 15:01 ` Christian König
2022-03-02 15:39 ` Alex Deucher
2022-03-02 16:17 ` [PATCH v2] drm/amdgpu: remove redundant null check Weiguo Li
2022-03-02 16:53 ` Christian König
2022-03-02 18:05 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.