2023-09-26 06:10:22

by Cong Liu

[permalink] [raw]
Subject: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message

This patch fixes a null pointer dereference in the error message that is
printed when the Display Core (DC) fails to initialize. The original
message includes the DC version number, which is undefined if the DC is
not initialized.

Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
Signed-off-by: Cong Liu <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8e98dda1e084..bf52a909f558 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
dce_version_to_string(adev->dm.dc->ctx->dce_version));
} else {
- DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
- dce_version_to_string(adev->dm.dc->ctx->dce_version));
+ DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);
goto error;
}

--
2.34.1


2023-09-26 18:47:44

by Harry Wentland

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message



On 2023-09-26 01:56, Cong Liu wrote:
> This patch fixes a null pointer dereference in the error message that is
> printed when the Display Core (DC) fails to initialize. The original
> message includes the DC version number, which is undefined if the DC is
> not initialized.
>
> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
> Signed-off-by: Cong Liu <[email protected]>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8e98dda1e084..bf52a909f558 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
> DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
> dce_version_to_string(adev->dm.dc->ctx->dce_version));
> } else {
> - DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
> - dce_version_to_string(adev->dm.dc->ctx->dce_version));
> + DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);

There is value in printing the version number. Let's not remove it.

Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.

Harry

> goto error;
> }
>

2023-09-27 02:50:15

by Cong Liu

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message


>
>
> On 2023-09-26 01:56, Cong Liu wrote:
>> This patch fixes a null pointer dereference in the error message that is
>> printed when the Display Core (DC) fails to initialize. The original
>> message includes the DC version number, which is undefined if the DC is
>> not initialized.
>>
>> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
>> Signed-off-by: Cong Liu <[email protected]>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 8e98dda1e084..bf52a909f558 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>> DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
>> dce_version_to_string(adev->dm.dc->ctx->dce_version));
>> } else {
>> - DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
>> - dce_version_to_string(adev->dm.dc->ctx->dce_version));
>> + DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);
>
> There is value in printing the version number. Let's not remove it.
>
> Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.

Hi Harry

I don't understand what you mean. Are you saying that I need to add a
NULL check in the if statement (i.e. if(adev->dm.dc &&
adev->dm.dc->ctx)), because adev->dm.dc is NULL in the else statement
and there is no way to print adev->dm.dc->ctx->dce_version.

Regards
Cong

>
> Harry
>
>> goto error;
>> }
>>
>

2023-09-27 11:46:30

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message

Am 26.09.23 um 15:09 schrieb Harry Wentland:
>
> On 2023-09-26 01:56, Cong Liu wrote:
>> This patch fixes a null pointer dereference in the error message that is
>> printed when the Display Core (DC) fails to initialize. The original
>> message includes the DC version number, which is undefined if the DC is
>> not initialized.
>>
>> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
>> Signed-off-by: Cong Liu <[email protected]>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 8e98dda1e084..bf52a909f558 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>> DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
>> dce_version_to_string(adev->dm.dc->ctx->dce_version));
>> } else {
>> - DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
>> - dce_version_to_string(adev->dm.dc->ctx->dce_version));
>> + DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);
> There is value in printing the version number. Let's not remove it.
>
> Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.

But as far as I understand it adev->dm.dc->ctx will always be NULL in
this case.

Regards,
Christian.

>
> Harry
>
>> goto error;
>> }
>>

2023-09-27 20:18:48

by Harry Wentland

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message



On 2023-09-27 10:07, Harry Wentland wrote:
>
>
> On 2023-09-27 01:23, Christian König wrote:
>> Am 26.09.23 um 15:09 schrieb Harry Wentland:
>>>
>>> On 2023-09-26 01:56, Cong Liu wrote:
>>>> This patch fixes a null pointer dereference in the error message that is
>>>> printed when the Display Core (DC) fails to initialize. The original
>>>> message includes the DC version number, which is undefined if the DC is
>>>> not initialized.
>>>>
>>>> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
>>>> Signed-off-by: Cong Liu <[email protected]>
>>>> ---
>>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> index 8e98dda1e084..bf52a909f558 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>>>           DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
>>>>                dce_version_to_string(adev->dm.dc->ctx->dce_version));
>>>>       } else {
>>>> -        DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
>>>> -             dce_version_to_string(adev->dm.dc->ctx->dce_version));
>>>> +        DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);
>>> There is value in printing the version number. Let's not remove it.
>>>
>>> Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.
>>
>> But as far as I understand it adev->dm.dc->ctx will always be NULL in this case.
>>
>
> Thanks, Christian. We're in the else of the NULL check for
> adev-dm.dc, so obviously we can't get the version. Silly me.
>
> Your patch is
> Reviewed-by: Harry Wentland <[email protected]>
>

Patch is merged.

Harry

> Harry
>
>> Regards,
>> Christian.
>>
>>>
>>> Harry
>>>
>>>>           goto error;
>>>>       }
>>>>  
>>
>

2023-09-28 02:52:57

by Harry Wentland

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message



On 2023-09-27 01:23, Christian König wrote:
> Am 26.09.23 um 15:09 schrieb Harry Wentland:
>>
>> On 2023-09-26 01:56, Cong Liu wrote:
>>> This patch fixes a null pointer dereference in the error message that is
>>> printed when the Display Core (DC) fails to initialize. The original
>>> message includes the DC version number, which is undefined if the DC is
>>> not initialized.
>>>
>>> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading DC")
>>> Signed-off-by: Cong Liu <[email protected]>
>>> ---
>>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 8e98dda1e084..bf52a909f558 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>>           DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
>>>                dce_version_to_string(adev->dm.dc->ctx->dce_version));
>>>       } else {
>>> -        DRM_INFO("Display Core v%s failed to initialize on %s\n", DC_VER,
>>> -             dce_version_to_string(adev->dm.dc->ctx->dce_version));
>>> +        DRM_INFO("Display Core failed to initialize with v%s!\n", DC_VER);
>> There is value in printing the version number. Let's not remove it.
>>
>> Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.
>
> But as far as I understand it adev->dm.dc->ctx will always be NULL in this case.
>

Thanks, Christian. We're in the else of the NULL check for
adev-dm.dc, so obviously we can't get the version. Silly me.

Your patch is
Reviewed-by: Harry Wentland <[email protected]>

Harry

> Regards,
> Christian.
>
>>
>> Harry
>>
>>>           goto error;
>>>       }
>>>  
>