Re: [PATCH] drm/msm: Fix NULL deref in adreno_load_gpu

[Archit Taneja <architt@codeaurora.org>]

On 12/15/2017 09:03 PM, Jordan Crouse wrote:
> On Thu, Dec 14, 2017 at 11:11:50AM +0530, Archit Taneja wrote:
>> The msm/kms driver should work even if there is no GPU device specified
>> in DT. Currently, we get a NULL dereference crash in adreno_load_gpu
>> since the driver assumes that priv->gpu_pdev is non-NULL.
>>
>> Perform an additional check on priv->gpu_pdev before trying to retrieve
>> the msm_gpu pointer from it.
>>
>> Fixes: eec874ce5ff1 (drm/msm/adreno: load gpu at probe/bind time)
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>>   drivers/gpu/drm/msm/adreno/adreno_device.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 05022ea2a007..ac60cf3c794e 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -124,10 +124,17 @@ const struct adreno_info *adreno_info(struct adreno_rev rev)
>>   struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
>>   {
>>   	struct msm_drm_private *priv = dev->dev_private;
>> -	struct platform_device *pdev = priv->gpu_pdev;
>> -	struct msm_gpu *gpu = platform_get_drvdata(priv->gpu_pdev);
>> +	struct platform_device *pdev;
>> +	struct msm_gpu *gpu;
>>   	int ret;
>>   
>> +	pdev = priv->gpu_pdev;
>> +	if (!pdev) {
>> +		dev_dbg(dev->dev, "no adreno platform device found\n");
>> +		return NULL;
>> +	}
>> +
>> +	gpu = platform_get_drvdata(pdev);
>>   	if (!gpu) {
>>   		dev_err(dev->dev, "no adreno device\n");
>>   		return NULL;
> 
> Obviously correct fix but I can't help but think that we should share the same
> error message, so something like:
> 
> struct msm_gpu *gpu = NULL;
> 
> ..
> 
> if (priv->gpu_pdev)
>     gpu = platform_get_drvdata(priv->gpu_pdev);
> 
> if (!gpu) {
> 	dev_err(dev->dev, "No GPU device was was found\n");
>   	return NULL;
> }
> 
> (also, I can't help but think maybe that dev_err should be a ONCE so you don't
> get a nasty message every time you open the file descriptor).

This approach looks better. I'll re-spin.

Thanks,
Archit

> 
> Jordan
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel