linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* BUG: amdgpu: NULL pointer dereference introduced in 5.9-rc1
@ 2020-09-30 20:13 Dirk Gouders
  2020-10-01 14:05 ` Alex Deucher
  0 siblings, 1 reply; 6+ messages in thread
From: Dirk Gouders @ 2020-09-30 20:13 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Christian König, linux-kernel, amd-gfx, Hawking Zhang, Evan Quan

Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
introduced a NULL pointer dereference when booting with
amdgpu.discovery=0.

For amdgpu.discovery=0 that commit effectively removed the call of
vega10_reg_base_init(adev), so I tested the correctness of the bisect
session by restoring that function call for amdgpu_discovery == 0 and with
that change, the NULL pointer dereference does not occur:

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 84d811b6e48b..2e93c5e1e7e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -699,7 +699,8 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
                                         "fallback to legacy init method\n");
                                vega10_reg_base_init(adev);
                        }
-               }
+               } else
+                       vega10_reg_base_init(adev);
                break;
        case CHIP_VEGA20:
                vega20_reg_base_init(adev);

Dirk

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: BUG: amdgpu: NULL pointer dereference introduced in 5.9-rc1
  2020-09-30 20:13 BUG: amdgpu: NULL pointer dereference introduced in 5.9-rc1 Dirk Gouders
@ 2020-10-01 14:05 ` Alex Deucher
  2020-10-01 19:55   ` [PATCH 0/1] drm/amdgpu: fix NULL pointer dereference for Renoir Dirk Gouders
  2020-10-01 19:55   ` [PATCH 1/1] " Dirk Gouders
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Deucher @ 2020-10-01 14:05 UTC (permalink / raw)
  To: Dirk Gouders
  Cc: Alex Deucher, Evan Quan, Hawking Zhang, Christian König,
	amd-gfx list, LKML

On Wed, Sep 30, 2020 at 4:46 PM Dirk Gouders <dirk@gouders.net> wrote:
>
> Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
> introduced a NULL pointer dereference when booting with
> amdgpu.discovery=0.
>
> For amdgpu.discovery=0 that commit effectively removed the call of
> vega10_reg_base_init(adev), so I tested the correctness of the bisect
> session by restoring that function call for amdgpu_discovery == 0 and with
> that change, the NULL pointer dereference does not occur:
>

Can I add your Signed-off-by?

Thanks,

Alex

> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 84d811b6e48b..2e93c5e1e7e6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -699,7 +699,8 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
>                                          "fallback to legacy init method\n");
>                                 vega10_reg_base_init(adev);
>                         }
> -               }
> +               } else
> +                       vega10_reg_base_init(adev);
>                 break;
>         case CHIP_VEGA20:
>                 vega20_reg_base_init(adev);
>
> Dirk
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 0/1] drm/amdgpu: fix NULL pointer dereference for Renoir
  2020-10-01 14:05 ` Alex Deucher
@ 2020-10-01 19:55   ` Dirk Gouders
  2020-10-01 19:55   ` [PATCH 1/1] " Dirk Gouders
  1 sibling, 0 replies; 6+ messages in thread
From: Dirk Gouders @ 2020-10-01 19:55 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Dirk Gouders, Alex Deucher, Christian König, linux-kernel,
	amd-gfx, Hawking Zhang, Evan Quan

Alex Deucher <alexdeucher@gmail.com> writes:

> On Wed, Sep 30, 2020 at 4:46 PM Dirk Gouders <dirk@gouders.net> wrote:
>>
>> Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
>> introduced a NULL pointer dereference when booting with
>> amdgpu.discovery=0.
>>
>> For amdgpu.discovery=0 that commit effectively removed the call of
>> vega10_reg_base_init(adev), so I tested the correctness of the bisect
>> session by restoring that function call for amdgpu_discovery == 0 and with
>> that change, the NULL pointer dereference does not occur:
>>
>
> Can I add your Signed-off-by?

I did not expect the diff to be seen as a proposed patch, not even that it
shows the correct fix.

Anyway, I did my best to create a hopefully acceptable patch with
some modification of the code that avoids "else" and an identical function call
at two places in the code.

I testet that patch with amdgpu.discovery={0,1} and together with the patch for the
first issue you helped me with.  The result is no more call traces.

Thank you for your patient assistance with the two issues.

Dirk


> Thanks,
>
> Alex
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> index 84d811b6e48b..2e93c5e1e7e6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> @@ -699,7 +699,8 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
>>                                          "fallback to legacy init method\n");
>>                                 vega10_reg_base_init(adev);
>>                         }
>> -               }
>> +               } else
>> +                       vega10_reg_base_init(adev);
>>                 break;
>>         case CHIP_VEGA20:
>>                 vega20_reg_base_init(adev);
>>
>> Dirk
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Dirk Gouders (1):
  drm/amdgpu: fix NULL pointer dereference for Renoir

 drivers/gpu/drm/amd/amdgpu/soc15.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/1] drm/amdgpu: fix NULL pointer dereference for Renoir
  2020-10-01 14:05 ` Alex Deucher
  2020-10-01 19:55   ` [PATCH 0/1] drm/amdgpu: fix NULL pointer dereference for Renoir Dirk Gouders
@ 2020-10-01 19:55   ` Dirk Gouders
  2020-10-01 20:30     ` Dirk Gouders
  1 sibling, 1 reply; 6+ messages in thread
From: Dirk Gouders @ 2020-10-01 19:55 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Dirk Gouders, Alex Deucher, Christian König, linux-kernel,
	amd-gfx, Hawking Zhang, Evan Quan

Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
introduced a NULL pointer dereference when booting with
amdgpu.discovery=0, because it removed the call of vega10_reg_base_init()
for that case.

Fix this by calling that funcion if amdgpu_discovery == 0 in addition to
the case that amdgpu_discovery_reg_base_init() failed.

Fixes: c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
Signed-off-by: Dirk Gouders <dirk@gouders.net>
Cc: Hawking Zhang <Hawking.Zhang@amd.com>
Cc: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/soc15.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 84d811b6e48b..f8cb62b326d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -694,12 +694,12 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
 		 * it doesn't support SRIOV. */
 		if (amdgpu_discovery) {
 			r = amdgpu_discovery_reg_base_init(adev);
-			if (r) {
-				DRM_WARN("failed to init reg base from ip discovery table, "
-					 "fallback to legacy init method\n");
-				vega10_reg_base_init(adev);
-			}
+			if (r == 0)
+			  break;
+			DRM_WARN("failed to init reg base from ip discovery table, "
+				 "fallback to legacy init method\n");
 		}
+		vega10_reg_base_init(adev);
 		break;
 	case CHIP_VEGA20:
 		vega20_reg_base_init(adev);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drm/amdgpu: fix NULL pointer dereference for Renoir
  2020-10-01 19:55   ` [PATCH 1/1] " Dirk Gouders
@ 2020-10-01 20:30     ` Dirk Gouders
  2020-10-01 20:47       ` Alex Deucher
  0 siblings, 1 reply; 6+ messages in thread
From: Dirk Gouders @ 2020-10-01 20:30 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Alex Deucher, Christian König, linux-kernel, amd-gfx,
	Hawking Zhang, Evan Quan

Dirk Gouders <dirk@gouders.net> writes:

> Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
> introduced a NULL pointer dereference when booting with
> amdgpu.discovery=0, because it removed the call of vega10_reg_base_init()
> for that case.
>
> Fix this by calling that funcion if amdgpu_discovery == 0 in addition to
> the case that amdgpu_discovery_reg_base_init() failed.
>
> Fixes: c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> Cc: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 84d811b6e48b..f8cb62b326d6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -694,12 +694,12 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
>  		 * it doesn't support SRIOV. */
>  		if (amdgpu_discovery) {
>  			r = amdgpu_discovery_reg_base_init(adev);
> -			if (r) {
> -				DRM_WARN("failed to init reg base from ip discovery table, "
> -					 "fallback to legacy init method\n");
> -				vega10_reg_base_init(adev);
> -			}
> +			if (r == 0)
> +			  break;

Grrr, wrong indentation here.
But I will wait for your review before v1.

Dirk


> +			DRM_WARN("failed to init reg base from ip discovery table, "
> +				 "fallback to legacy init method\n");
>  		}
> +		vega10_reg_base_init(adev);
>  		break;
>  	case CHIP_VEGA20:
>  		vega20_reg_base_init(adev);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] drm/amdgpu: fix NULL pointer dereference for Renoir
  2020-10-01 20:30     ` Dirk Gouders
@ 2020-10-01 20:47       ` Alex Deucher
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2020-10-01 20:47 UTC (permalink / raw)
  To: Dirk Gouders
  Cc: Alex Deucher, Christian König, LKML, amd-gfx list,
	Hawking Zhang, Evan Quan

On Thu, Oct 1, 2020 at 4:33 PM Dirk Gouders <dirk@gouders.net> wrote:
>
> Dirk Gouders <dirk@gouders.net> writes:
>
> > Commit c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
> > introduced a NULL pointer dereference when booting with
> > amdgpu.discovery=0, because it removed the call of vega10_reg_base_init()
> > for that case.
> >
> > Fix this by calling that funcion if amdgpu_discovery == 0 in addition to
> > the case that amdgpu_discovery_reg_base_init() failed.
> >
> > Fixes: c1cf79ca5ced46 (drm/amdgpu: use IP discovery table for renoir)
> > Signed-off-by: Dirk Gouders <dirk@gouders.net>
> > Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> > Cc: Evan Quan <evan.quan@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/soc15.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > index 84d811b6e48b..f8cb62b326d6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > @@ -694,12 +694,12 @@ static void soc15_reg_base_init(struct amdgpu_device *adev)
> >                * it doesn't support SRIOV. */
> >               if (amdgpu_discovery) {
> >                       r = amdgpu_discovery_reg_base_init(adev);
> > -                     if (r) {
> > -                             DRM_WARN("failed to init reg base from ip discovery table, "
> > -                                      "fallback to legacy init method\n");
> > -                             vega10_reg_base_init(adev);
> > -                     }
> > +                     if (r == 0)
> > +                       break;
>
> Grrr, wrong indentation here.
> But I will wait for your review before v1.

Fixed up locally and applied.  Thanks!

Alex


>
> Dirk
>
>
> > +                     DRM_WARN("failed to init reg base from ip discovery table, "
> > +                              "fallback to legacy init method\n");
> >               }
> > +             vega10_reg_base_init(adev);
> >               break;
> >       case CHIP_VEGA20:
> >               vega20_reg_base_init(adev);

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-01 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30 20:13 BUG: amdgpu: NULL pointer dereference introduced in 5.9-rc1 Dirk Gouders
2020-10-01 14:05 ` Alex Deucher
2020-10-01 19:55   ` [PATCH 0/1] drm/amdgpu: fix NULL pointer dereference for Renoir Dirk Gouders
2020-10-01 19:55   ` [PATCH 1/1] " Dirk Gouders
2020-10-01 20:30     ` Dirk Gouders
2020-10-01 20:47       ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).