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

Andrzej Pietrasiewicz
2019-12-16T20:37:17Z

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