diff mbox series

[2/2] drm/amd/display: Fix null pointer dereference in error message

Message ID 20230926055618.119195-1-liucong2@kylinos.cn
State New
Headers
Series [1/2] drm/amd/pm: Fix warn: missing error code in smu_v13_0_0_check_ecc_table_support() |

Commit Message

Cong Liu Sept. 26, 2023, 5:56 a.m. UTC 
  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 <liucong2@kylinos.cn>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Harry Wentland Sept. 26, 2023, 1:09 p.m. UTC | #1 
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 <liucong2@kylinos.cn>
> ---
>  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;
>  	}
>
Cong Liu Sept. 27, 2023, 2:15 a.m. UTC | #2 
> 
> 
> 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 <liucong2@kylinos.cn>
>> ---
>>   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;
>>   	}
>>   
>
Christian König Sept. 27, 2023, 5:23 a.m. UTC | #3 
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 <liucong2@kylinos.cn>
>> ---
>>   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;
>>   	}
>>
Harry Wentland Sept. 27, 2023, 2:07 p.m. UTC | #4 
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 <liucong2@kylinos.cn>
>>> ---
>>>   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 <harry.wentland@amd.com>

Harry

> Regards,
> Christian.
> 
>>
>> Harry
>>
>>>           goto error;
>>>       }
>>>   
>
Harry Wentland Sept. 27, 2023, 5:44 p.m. UTC | #5 
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 <liucong2@kylinos.cn>
>>>> ---
>>>>   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 <harry.wentland@amd.com>
> 

Patch is merged.

Harry

> Harry
> 
>> Regards,
>> Christian.
>>
>>>
>>> Harry
>>>
>>>>           goto error;
>>>>       }
>>>>   
>>
>
diff mbox series

Patch

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;
 	}