From: Charan Teja Reddy <charante(a)codeaurora.org>
[ Upstream commit f492283b157053e9555787262f058ae33096f568 ]
It is expected from the clients to follow the below steps on an imported
dmabuf fd:
a) dmabuf = dma_buf_get(fd) // Get the dmabuf from fd
b) dma_buf_attach(dmabuf); // Clients attach to the dmabuf
o Here the kernel does some slab allocations, say for
dma_buf_attachment and may be some other slab allocation in the
dmabuf->ops->attach().
c) Client may need to do dma_buf_map_attachment().
d) Accordingly dma_buf_unmap_attachment() should be called.
e) dma_buf_detach () // Clients detach to the dmabuf.
o Here the slab allocations made in b) are freed.
f) dma_buf_put(dmabuf) // Can free the dmabuf if it is the last
reference.
Now say an erroneous client failed at step c) above thus it directly
called dma_buf_put(), step f) above. Considering that it may be the last
reference to the dmabuf, buffer will be freed with pending attachments
left to the dmabuf which can show up as the 'memory leak'. This should
at least be reported as the WARN().
Signed-off-by: Charan Teja Reddy <charante(a)codeaurora.org>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1627043468-16381-1-git-send-e…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/dma-buf/dma-buf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 758de0e9b2ddc..16bbc9bc9e6d1 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -79,6 +79,7 @@ static void dma_buf_release(struct dentry *dentry)
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
dma_resv_fini(dmabuf->resv);
+ WARN_ON(!list_empty(&dmabuf->attachments));
module_put(dmabuf->owner);
kfree(dmabuf->name);
kfree(dmabuf);
--
2.33.0
From: Charan Teja Reddy <charante(a)codeaurora.org>
[ Upstream commit f492283b157053e9555787262f058ae33096f568 ]
It is expected from the clients to follow the below steps on an imported
dmabuf fd:
a) dmabuf = dma_buf_get(fd) // Get the dmabuf from fd
b) dma_buf_attach(dmabuf); // Clients attach to the dmabuf
o Here the kernel does some slab allocations, say for
dma_buf_attachment and may be some other slab allocation in the
dmabuf->ops->attach().
c) Client may need to do dma_buf_map_attachment().
d) Accordingly dma_buf_unmap_attachment() should be called.
e) dma_buf_detach () // Clients detach to the dmabuf.
o Here the slab allocations made in b) are freed.
f) dma_buf_put(dmabuf) // Can free the dmabuf if it is the last
reference.
Now say an erroneous client failed at step c) above thus it directly
called dma_buf_put(), step f) above. Considering that it may be the last
reference to the dmabuf, buffer will be freed with pending attachments
left to the dmabuf which can show up as the 'memory leak'. This should
at least be reported as the WARN().
Signed-off-by: Charan Teja Reddy <charante(a)codeaurora.org>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1627043468-16381-1-git-send-e…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/dma-buf/dma-buf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 922416b3aaceb..93e9bf7382595 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -79,6 +79,7 @@ static void dma_buf_release(struct dentry *dentry)
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
dma_resv_fini(dmabuf->resv);
+ WARN_ON(!list_empty(&dmabuf->attachments));
module_put(dmabuf->owner);
kfree(dmabuf->name);
kfree(dmabuf);
--
2.33.0
From: Charan Teja Reddy <charante(a)codeaurora.org>
[ Upstream commit f492283b157053e9555787262f058ae33096f568 ]
It is expected from the clients to follow the below steps on an imported
dmabuf fd:
a) dmabuf = dma_buf_get(fd) // Get the dmabuf from fd
b) dma_buf_attach(dmabuf); // Clients attach to the dmabuf
o Here the kernel does some slab allocations, say for
dma_buf_attachment and may be some other slab allocation in the
dmabuf->ops->attach().
c) Client may need to do dma_buf_map_attachment().
d) Accordingly dma_buf_unmap_attachment() should be called.
e) dma_buf_detach () // Clients detach to the dmabuf.
o Here the slab allocations made in b) are freed.
f) dma_buf_put(dmabuf) // Can free the dmabuf if it is the last
reference.
Now say an erroneous client failed at step c) above thus it directly
called dma_buf_put(), step f) above. Considering that it may be the last
reference to the dmabuf, buffer will be freed with pending attachments
left to the dmabuf which can show up as the 'memory leak'. This should
at least be reported as the WARN().
Signed-off-by: Charan Teja Reddy <charante(a)codeaurora.org>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1627043468-16381-1-git-send-e…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/dma-buf/dma-buf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 511fe0d217a08..733c8b1c8467c 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -79,6 +79,7 @@ static void dma_buf_release(struct dentry *dentry)
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
dma_resv_fini(dmabuf->resv);
+ WARN_ON(!list_empty(&dmabuf->attachments));
module_put(dmabuf->owner);
kfree(dmabuf->name);
kfree(dmabuf);
--
2.33.0
From: Charan Teja Reddy <charante(a)codeaurora.org>
[ Upstream commit f492283b157053e9555787262f058ae33096f568 ]
It is expected from the clients to follow the below steps on an imported
dmabuf fd:
a) dmabuf = dma_buf_get(fd) // Get the dmabuf from fd
b) dma_buf_attach(dmabuf); // Clients attach to the dmabuf
o Here the kernel does some slab allocations, say for
dma_buf_attachment and may be some other slab allocation in the
dmabuf->ops->attach().
c) Client may need to do dma_buf_map_attachment().
d) Accordingly dma_buf_unmap_attachment() should be called.
e) dma_buf_detach () // Clients detach to the dmabuf.
o Here the slab allocations made in b) are freed.
f) dma_buf_put(dmabuf) // Can free the dmabuf if it is the last
reference.
Now say an erroneous client failed at step c) above thus it directly
called dma_buf_put(), step f) above. Considering that it may be the last
reference to the dmabuf, buffer will be freed with pending attachments
left to the dmabuf which can show up as the 'memory leak'. This should
at least be reported as the WARN().
Signed-off-by: Charan Teja Reddy <charante(a)codeaurora.org>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1627043468-16381-1-git-send-e…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Sasha Levin <sashal(a)kernel.org>
---
drivers/dma-buf/dma-buf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 63d32261b63ff..474de2d988ca7 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -82,6 +82,7 @@ static void dma_buf_release(struct dentry *dentry)
if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
dma_resv_fini(dmabuf->resv);
+ WARN_ON(!list_empty(&dmabuf->attachments));
module_put(dmabuf->owner);
kfree(dmabuf->name);
kfree(dmabuf);
--
2.33.0
From: Michel Dänzer <mdaenzer(a)redhat.com>
This makes sure we don't hit the
BUG_ON(dmabuf->cb_in.active || dmabuf->cb_out.active);
in dma_buf_release, which could be triggered by user space closing the
dma-buf file description while there are outstanding fence callbacks
from dma_buf_poll.
Cc: stable(a)vger.kernel.org
Signed-off-by: Michel Dänzer <mdaenzer(a)redhat.com>
---
drivers/dma-buf/dma-buf.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 6c520c9bd93c..ec25498a971f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -65,12 +65,9 @@ static void dma_buf_release(struct dentry *dentry)
BUG_ON(dmabuf->vmapping_counter);
/*
- * Any fences that a dma-buf poll can wait on should be signaled
- * before releasing dma-buf. This is the responsibility of each
- * driver that uses the reservation objects.
- *
- * If you hit this BUG() it means someone dropped their ref to the
- * dma-buf while still having pending operation to the buffer.
+ * If you hit this BUG() it could mean:
+ * * There's a file reference imbalance in dma_buf_poll / dma_buf_poll_cb or somewhere else
+ * * dmabuf->cb_in/out.active are non-0 despite no pending fence callback
*/
BUG_ON(dmabuf->cb_in.active || dmabuf->cb_out.active);
@@ -196,6 +193,7 @@ static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)
static void dma_buf_poll_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
{
struct dma_buf_poll_cb_t *dcb = (struct dma_buf_poll_cb_t *)cb;
+ struct dma_buf *dmabuf = container_of(dcb->poll, struct dma_buf, poll);
unsigned long flags;
spin_lock_irqsave(&dcb->poll->lock, flags);
@@ -203,6 +201,8 @@ static void dma_buf_poll_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
dcb->active = 0;
spin_unlock_irqrestore(&dcb->poll->lock, flags);
dma_fence_put(fence);
+ /* Paired with get_file in dma_buf_poll */
+ fput(dmabuf->file);
}
static bool dma_buf_poll_shared(struct dma_resv *resv,
@@ -278,6 +278,9 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
spin_unlock_irq(&dmabuf->poll.lock);
if (events & EPOLLOUT) {
+ /* Paired with fput in dma_buf_poll_cb */
+ get_file(dmabuf->file);
+
if (!dma_buf_poll_shared(resv, dcb) &&
!dma_buf_poll_excl(resv, dcb))
/* No callback queued, wake up any other waiters */
@@ -299,6 +302,9 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
spin_unlock_irq(&dmabuf->poll.lock);
if (events & EPOLLIN) {
+ /* Paired with fput in dma_buf_poll_cb */
+ get_file(dmabuf->file);
+
if (!dma_buf_poll_excl(resv, dcb))
/* No callback queued, wake up any other waiters */
dma_buf_poll_cb(NULL, &dcb->cb);
--
2.32.0
Am 29.10.21 um 04:15 schrieb guangming.cao(a)mediatek.com:
> From: Guangming Cao <Guangming.Cao(a)mediatek.com>
>
> On Fri, 2021-10-08 at 12:24 +0200, Christian König wrote:
>> Am 08.10.21 um 09:54 schrieb guangming.cao(a)mediatek.com:
>>> From: Guangming Cao <Guangming.Cao(a)mediatek.com>
>>>
>>> Because dma-buf.name can be freed in func: "dma_buf_set_name",
>>> so, we need to acquire lock first before we read/write dma_buf.name
>>> to prevent Use After Free(UAF) issue.
>>>
>>> Signed-off-by: Guangming Cao <Guangming.Cao(a)mediatek.com>
>> Reviewed-by: Christian König <christian.koenig(a)amd.com>
>>
>> Going to push that upstream if nobody else objects.
>>
>> Thanks,
>> Christian.
> Just a gentle ping for this patch, please kindly let me know how is it going.
Ah, yes. Thanks for the reminder.
I've just pushed this to drm-misc-fixes.
Christian.
>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 511fe0d217a0..a7f6fd13a635 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1372,6 +1372,8 @@ static int dma_buf_debug_show(struct seq_file
>>> *s, void *unused)
>>> if (ret)
>>> goto error_unlock;
>>>
>>> +
>>> + spin_lock(&buf_obj->name_lock);
>>> seq_printf(s,
>>> "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n",
>>> buf_obj->size,
>>> buf_obj->file->f_flags, buf_obj->file-
>>>> f_mode,
>>> @@ -1379,6 +1381,7 @@ static int dma_buf_debug_show(struct seq_file
>>> *s, void *unused)
>>> buf_obj->exp_name,
>>> file_inode(buf_obj->file)->i_ino,
>>> buf_obj->name ?: "");
>>> + spin_unlock(&buf_obj->name_lock);
>>>
>>> robj = buf_obj->resv;
>>> fence = dma_resv_excl_fence(robj);
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek(a)lists.infradead.org
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infr…
Hi guys,
a few more bug fixes, looks like the more selftests I add the more odies I find.
Assuming the CI tests now pass I will start pushing patches I've already got an rb for to drm-misc-next.
Please review and/or comment,
Christian.
Am 26.10.21 um 13:52 schrieb guangming.cao(a)mediatek.com:
> From: Guangming Cao <Guangming.Cao(a)mediatek.com>
>
> On Tue, 2021-10-26 at 13:18 +0200, Christian König wrote:
>> Am 14.10.21 um 12:25 schrieb guangming.cao(a)mediatek.com:
>>> From: Guangming Cao <Guangming.Cao(a)mediatek.com>
>>>
>>> In this patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat…;reserved=0),
>>> it add a new IOCTL to support dma-buf user to set debug name.
>>>
>>> But it also added a limitation of this IOCTL, it needs the
>>> attachments of dmabuf should be empty, otherwise it will fail.
>>>
>>> For the original series, the idea was that allowing name change
>>> mid-use could confuse the users about the dma-buf.
>>> However, the rest of the series also makes sure each dma-buf have a
>>> unique
>>> inode(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat…;reserved=0), and any
>>> accounting
>>> should probably use that, without relying on the name as much.
>>>
>>> So, removing this restriction will let dma-buf userspace users to
>>> use it
>>> more comfortably and without any side effect.
>>>
>>> Signed-off-by: Guangming Cao <Guangming.Cao(a)mediatek.com>
>> We could now cleanup the return value from dma_buf_set_name() into a
>> void since that function can't fail any more as far as I can see.
>>
>> But that isn't mandatory I think, patch is Reviewed-by: Christian
>> König
>> <christian.koenig(a)amd.com>
>>
> So, here is no need to check return value of 'strndup_user',
> just return without error code if the almost impossible error occurs?
Good point, totally missed that one.
In that case I'm going to push the patch to drm-misc-next as is.
Regards,
Christian.
>
> Guangming.
>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 17 +++--------------
>>> 1 file changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 511fe0d217a0..5fbb3a2068a3 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>>
>>> /**
>>> * dma_buf_set_name - Set a name to a specific dma_buf to track
>>> the usage.
>>> - * The name of the dma-buf buffer can only be set when the dma-buf
>>> is not
>>> - * attached to any devices. It could theoritically support
>>> changing the
>>> - * name of the dma-buf if the same piece of memory is used for
>>> multiple
>>> - * purpose between different devices.
>>> + * It could support changing the name of the dma-buf if the same
>>> + * piece of memory is used for multiple purpose between different
>>> devices.
>>> *
>>> * @dmabuf: [in] dmabuf buffer that will be renamed.
>>> * @buf: [in] A piece of userspace memory that contains
>>> the name of
>>> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file
>>> *file, poll_table *poll)
>>> static long dma_buf_set_name(struct dma_buf *dmabuf, const char
>>> __user *buf)
>>> {
>>> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
>>> - long ret = 0;
>>>
>>> if (IS_ERR(name))
>>> return PTR_ERR(name);
>>>
>>> - dma_resv_lock(dmabuf->resv, NULL);
>>> - if (!list_empty(&dmabuf->attachments)) {
>>> - ret = -EBUSY;
>>> - kfree(name);
>>> - goto out_unlock;
>>> - }
>>> spin_lock(&dmabuf->name_lock);
>>> kfree(dmabuf->name);
>>> dmabuf->name = name;
>>> spin_unlock(&dmabuf->name_lock);
>>>
>>> -out_unlock:
>>> - dma_resv_unlock(dmabuf->resv);
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static long dma_buf_ioctl(struct file *file,
>>
Am 14.10.21 um 12:25 schrieb guangming.cao(a)mediatek.com:
> From: Guangming Cao <Guangming.Cao(a)mediatek.com>
>
> In this patch(https://patchwork.freedesktop.org/patch/310349),
> it add a new IOCTL to support dma-buf user to set debug name.
>
> But it also added a limitation of this IOCTL, it needs the
> attachments of dmabuf should be empty, otherwise it will fail.
>
> For the original series, the idea was that allowing name change
> mid-use could confuse the users about the dma-buf.
> However, the rest of the series also makes sure each dma-buf have a unique
> inode(https://patchwork.freedesktop.org/patch/310387/), and any accounting
> should probably use that, without relying on the name as much.
>
> So, removing this restriction will let dma-buf userspace users to use it
> more comfortably and without any side effect.
>
> Signed-off-by: Guangming Cao <Guangming.Cao(a)mediatek.com>
We could now cleanup the return value from dma_buf_set_name() into a
void since that function can't fail any more as far as I can see.
But that isn't mandatory I think, patch is Reviewed-by: Christian König
<christian.koenig(a)amd.com>
Regards,
Christian.
> ---
> drivers/dma-buf/dma-buf.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 511fe0d217a0..5fbb3a2068a3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,10 +325,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>
> /**
> * dma_buf_set_name - Set a name to a specific dma_buf to track the usage.
> - * The name of the dma-buf buffer can only be set when the dma-buf is not
> - * attached to any devices. It could theoritically support changing the
> - * name of the dma-buf if the same piece of memory is used for multiple
> - * purpose between different devices.
> + * It could support changing the name of the dma-buf if the same
> + * piece of memory is used for multiple purpose between different devices.
> *
> * @dmabuf: [in] dmabuf buffer that will be renamed.
> * @buf: [in] A piece of userspace memory that contains the name of
> @@ -341,25 +339,16 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
> {
> char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
> - long ret = 0;
>
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> - dma_resv_lock(dmabuf->resv, NULL);
> - if (!list_empty(&dmabuf->attachments)) {
> - ret = -EBUSY;
> - kfree(name);
> - goto out_unlock;
> - }
> spin_lock(&dmabuf->name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
> spin_unlock(&dmabuf->name_lock);
>
> -out_unlock:
> - dma_resv_unlock(dmabuf->resv);
> - return ret;
> + return 0;
> }
>
> static long dma_buf_ioctl(struct file *file,
Am 26.10.21 um 10:34 schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd(a)arndb.de>
>
> The new driver incorrectly unwinds after errors, as clang points out:
>
> drivers/dma-buf/st-dma-resv.c:295:7: error: variable 'i' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> if (r) {
> ^
> drivers/dma-buf/st-dma-resv.c:336:9: note: uninitialized use occurs here
> while (i--)
> ^
> drivers/dma-buf/st-dma-resv.c:295:3: note: remove the 'if' if its condition is always false
> if (r) {
> ^~~~~~~~
> drivers/dma-buf/st-dma-resv.c:288:6: error: variable 'i' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> if (r) {
> ^
> drivers/dma-buf/st-dma-resv.c:336:9: note: uninitialized use occurs here
> while (i--)
> ^
> drivers/dma-buf/st-dma-resv.c:288:2: note: remove the 'if' if its condition is always false
> if (r) {
> ^~~~~~~~
> drivers/dma-buf/st-dma-resv.c:280:10: note: initialize the variable 'i' to silence this warning
> int r, i;
> ^
> = 0
>
> Skip cleaning up the bits that have not been allocated at this point.
>
> Fixes: 1d51775cd3f5 ("dma-buf: add dma_resv selftest v4")
> Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
I already send out a patch to fix this up, but forgot to fix both gotos.
Going to add my rb and using that one here instead.
Thanks,
Christian.
> ---
> I'm not familiar with these interfaces, so I'm just guessing where
> we should jump after an error, please double-check and fix if necessary.
> ---
> drivers/dma-buf/st-dma-resv.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c
> index 6f3ba756da3e..bc32b3eedcb6 100644
> --- a/drivers/dma-buf/st-dma-resv.c
> +++ b/drivers/dma-buf/st-dma-resv.c
> @@ -287,7 +287,7 @@ static int test_get_fences(void *arg, bool shared)
> r = dma_resv_lock(&resv, NULL);
> if (r) {
> pr_err("Resv locking failed\n");
> - goto err_free;
> + goto err_resv;
> }
>
> if (shared) {
> @@ -295,7 +295,7 @@ static int test_get_fences(void *arg, bool shared)
> if (r) {
> pr_err("Resv shared slot allocation failed\n");
> dma_resv_unlock(&resv);
> - goto err_free;
> + goto err_resv;
> }
>
> dma_resv_add_shared_fence(&resv, f);
> @@ -336,6 +336,7 @@ static int test_get_fences(void *arg, bool shared)
> while (i--)
> dma_fence_put(fences[i]);
> kfree(fences);
> +err_resv:
> dma_resv_fini(&resv);
> dma_fence_put(f);
> return r;