[bug report] drm/gem: Add drm_gem_object_funcs

[bug report] drm/gem: Add drm_gem_object_funcs

[Dan Carpenter]

Hello Noralf Trønnes,

The patch b39b5394fabc: "drm/gem: Add drm_gem_object_funcs" from Nov
10, 2018, leads to the following Smatch static checker warning:

	drivers/gpu/drm/drm_client.c:240 drm_client_buffer_delete()
	warn: variable dereferenced before check 'buffer->gem' (see line 238)

drivers/gpu/drm/drm_client.c
    234 static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
    235 {
    236         struct drm_device *dev = buffer->client->dev;
    237 
    238         drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
                                        ^^^^^^^^^^^
Unchecked dereference inside function call.

    239 
--> 240         if (buffer->gem)
                    ^^^^^^^^^^^
Too late.  I don't honestly, know why this warning is only showing up
now four years later.  Perhaps Smatch thought buffer->gem was always
non-NULL before but now it's not sure.  I've added Dmitry to the CC list
because he seems to has been working in this area recently and might
know what's going on.

    241                 drm_gem_object_put(buffer->gem);
    242 
    243         if (buffer->handle)
    244                 drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
    245 
    246         kfree(buffer);
    247 }

regards,
dan carpenter

Re: [bug report] drm/gem: Add drm_gem_object_funcs

[Dmitry Osipenko]

On 10/26/22 12:59, Dan Carpenter wrote:
> Hello Noralf Trønnes,
> 
> The patch b39b5394fabc: "drm/gem: Add drm_gem_object_funcs" from Nov
> 10, 2018, leads to the following Smatch static checker warning:
> 
> 	drivers/gpu/drm/drm_client.c:240 drm_client_buffer_delete()
> 	warn: variable dereferenced before check 'buffer->gem' (see line 238)
> 
> drivers/gpu/drm/drm_client.c
>     234 static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>     235 {
>     236         struct drm_device *dev = buffer->client->dev;
>     237 
>     238         drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
>                                         ^^^^^^^^^^^
> Unchecked dereference inside function call.
> 
>     239 
> --> 240         if (buffer->gem)
>                     ^^^^^^^^^^^
> Too late.  I don't honestly, know why this warning is only showing up
> now four years later.  Perhaps Smatch thought buffer->gem was always
> non-NULL before but now it's not sure.  I've added Dmitry to the CC list
> because he seems to has been working in this area recently and might
> know what's going on.

This is a valid bug. Before my changes drm_gem_vunmap() wasn't touching
buffer->gem when buffer->map=NULL, now it will. I'll prepare the fix,
thank you for the report.

-- 
Best regards,
Dmitry