[PATCH v2 4/4] drm: NULL pointer dereference [null-pointer-deref] (CWE 476) problem

[Joe Moriarty]

On 2/19/2018 8:32 AM, Daniel Vetter wrote:
> On Mon, Feb 12, 2018 at 02:51:44PM -0500, Joe Moriarty wrote:
>> The Parfait (version 2.1.0) static code analysis tool found the
>> following NULL pointer derefernce problem.
>>
>> - drivers/gpu/drm/drm_vblank.c
>> Null pointer checks were added to return values from calls to
>> drm_crtc_from_index().  There is a possibility, however minute, that
>> crtc->index may not be found when trying to find the struct crtc
>> from it's assigned index given in drm_crtc_init_with_planes().
>> 3 return checks for NULL where added.
>>
>> Signed-off-by: Joe Moriarty <joe.moriarty at oracle.com>
>> Reviewed-by: Steven Sistare <steven.sistare at oracle.com>
> 
> These are all drivers bugs, we'd need at least a WARN_ON when the crtc
> doesn't exist. Otherwise this would just silently paper over a fairly
> serious kernel bug (which doesn't improve things really).
> 
> Something like
> 
> 	if (WARN_ON(!crtc))
> 		return NULL;
> 
> is what I'd go with.
> 
> -Daniel
>
I will make the requested changes and resubmit the patch.  Thanks for 
reviewing the patches.

Joe

>> ---
>>   drivers/gpu/drm/drm_vblank.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 32d9bcf5be7f..a3a1bce87468 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -120,7 +120,7 @@ static u32 __get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>>   	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>>   		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>>   
>> -		if (crtc->funcs->get_vblank_counter)
>> +		if (crtc && crtc->funcs->get_vblank_counter)
>>   			return crtc->funcs->get_vblank_counter(crtc);
>>   	}
>>   
>> @@ -318,7 +318,7 @@ static void __disable_vblank(struct drm_device *dev, unsigned int pipe)
>>   	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>>   		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>>   
>> -		if (crtc->funcs->disable_vblank) {
>> +		if (crtc && crtc->funcs->disable_vblank) {
>>   			crtc->funcs->disable_vblank(crtc);
>>   			return;
>>   		}
>> @@ -918,7 +918,7 @@ static int __enable_vblank(struct drm_device *dev, unsigned int pipe)
>>   	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>>   		struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>>   
>> -		if (crtc->funcs->enable_vblank)
>> +		if (crtc && crtc->funcs->enable_vblank)
>>   			return crtc->funcs->enable_vblank(crtc);
>>   	}
>>   
>> -- 
>> 2.15.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>