Tim Gardner
2021-Sep-20 18:16 UTC
[Nouveau] [PATCH] drm/nouveau/ga102: Free resources on error in ga102_chan_new()
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.
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: David Airlie <airlied at linux.ie>
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: Karol Herbst <kherbst at redhat.com>
Cc: dri-devel at lists.freedesktop.org
Cc: nouveau at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner at 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
Karol Herbst
2021-Sep-21 02:07 UTC
[Nouveau] [PATCH] drm/nouveau/ga102: Free resources on error in ga102_chan_new()
On Mon, Sep 20, 2021 at 8:17 PM Tim Gardner <tim.gardner at 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.> Cc: Ben Skeggs <bskeggs at redhat.com> > Cc: David Airlie <airlied at linux.ie> > Cc: Daniel Vetter <daniel at ffwll.ch> > Cc: Karol Herbst <kherbst at redhat.com> > Cc: dri-devel at lists.freedesktop.org > Cc: nouveau at lists.freedesktop.org > Cc: linux-kernel at vger.kernel.org > Signed-off-by: Tim Gardner <tim.gardner at 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 >