[PATCHv4 03/36] drm/gem-fb-helper: Allow drivers to allocate struct drm_framebuffer on their own

[Andrzej Pietrasiewicz]

Hi Liviu,

My way of thinking is explained below. Do you still find it problematic?

W dniu 16.12.2019 o 18:08, Liviu Dudau pisze:
> Hi Andrzej,
> 

<snip>

>> +struct drm_framebuffer *
>> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
>> +			     const struct drm_framebuffer_funcs *funcs)
>> +{
>> +	struct drm_gem_object *objs[4];
>> +	struct drm_framebuffer *fb;
>> +	int ret, num_planes;
>> +
>> +	ret = drm_gem_fb_lookup(dev, file, mode_cmd, objs);
>> +	if (ret < 0)
>> +		return ERR_PTR(ret);

here objs is guaranteed to have been filled...

>> +	num_planes = ret;
>> +
>> +	ret = drm_gem_fb_size_check(dev, mode_cmd, objs);
> 
> if drm_gem_fb_size_check() returns an error, then ...
> 
>> +	if (ret)
>> +		fb = ERR_PTR(ret);
>> +	else
>> +		fb = drm_gem_fb_alloc(dev, mode_cmd, objs, num_planes, funcs);
> 
> .... the else part is not taken, but ...

... nonetheless objs have already been looked up...

> 
>>   
>> -	return ERR_PTR(ret);
>> +	if (IS_ERR(fb))
>> +		for (num_planes--; num_planes >= 0; num_planes--)
>> +			drm_gem_object_put_unlocked(objs[num_planes]);
> 
> ... here you'll attempt to dereference the objs. I don't think that is correct.

... so it is safe to dereference objs here

Regards,

Andrzej