lkml.org 
[lkml]   [2021]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] drm/nouveau/ga102: Free resources on error in ga102_chan_new()
From
Date


On 9/20/21 8:07 PM, Karol Herbst wrote:
> On Mon, Sep 20, 2021 at 8:17 PM Tim Gardner <tim.gardner@canonical.com> wrote:
>>
>> Coverity complains of a resource leak in ga102_chan_new():
>>
>> CID 119637 (#7 of 7): Resource leak (RESOURCE_LEAK)
>> 13. leaked_storage: Variable chan going out of scope leaks the storage it points to.
>> 190 return ret;
>>
>> Fix this by freeing 'chan' in the error path.
>>
>
> yeah, this is actually a false positive. I ran your patch through
> kasan and got a use-after-free as we deallocate the passed in pointer
> after calling the function pointer to the new function. One might
> argue that the programming style isn't the best and we should be
> explicit about freeing memory though.
>

So the caller of this constructor has to look at the error return code
and decide whether the value stored in *pobject can be freed ? I guess
if the caller initializes the value at *pobject to be NULL then it can
kfree() regardless.

>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Karol Herbst <kherbst@redhat.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: nouveau@lists.freedesktop.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>> ---
>> .../gpu/drm/nouveau/nvkm/engine/fifo/ga102.c | 20 ++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga102.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga102.c
>> index f897bef13acf..4dbdfb53e65f 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga102.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga102.c
>> @@ -175,19 +175,21 @@ ga102_chan_new(struct nvkm_device *device,
>> }
>> }
>>
>> - if (!chan->ctrl.runl)
>> - return -ENODEV;
>> + if (!chan->ctrl.runl) {
>> + ret = -ENODEV;
>> + goto free_chan;
>> + }
>>
>> chan->ctrl.chan = nvkm_rd32(device, chan->ctrl.runl + 0x004) & 0xfffffff0;
>> args->token = nvkm_rd32(device, chan->ctrl.runl + 0x008) & 0xffff0000;
>>
>> ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 0x1000, true, &chan->mthd);
>> if (ret)
>> - return ret;
>> + goto free_chan;
>>
>> ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 0x1000, true, &chan->inst);
>> if (ret)
>> - return ret;
>> + goto free_chan;
>>
>> nvkm_kmap(chan->inst);
>> nvkm_wo32(chan->inst, 0x010, 0x0000face);
>> @@ -209,11 +211,11 @@ ga102_chan_new(struct nvkm_device *device,
>>
>> ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 0x1000, true, &chan->user);
>> if (ret)
>> - return ret;
>> + goto free_chan;
>>
>> ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 0x1000, true, &chan->runl);
>> if (ret)
>> - return ret;
>> + goto free_chan;
>>
>> nvkm_kmap(chan->runl);
>> nvkm_wo32(chan->runl, 0x00, 0x80030001);
>> @@ -228,10 +230,14 @@ ga102_chan_new(struct nvkm_device *device,
>>
>> ret = nvkm_vmm_join(vmm, chan->inst);
>> if (ret)
>> - return ret;
>> + goto free_chan;
>>
>> chan->vmm = nvkm_vmm_ref(vmm);
>> return 0;
>> +
>> +free_chan:
>> + kfree(chan);
>> + return ret;
>> }
>>
>> static const struct nvkm_device_oclass
>> --
>> 2.33.0
>>
>

--
-----------
Tim Gardner
Canonical, Inc

\
 
 \ /
  Last update: 2021-09-21 15:22    [W:0.049 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site