* Re: [PATCH] drivers/base: wrong return value of dmam_declare_coherent_memory
@ 2016-04-28 14:31 Andrey Gursky
2016-04-28 15:11 ` Vyacheslav Yurkov
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Gursky @ 2016-04-28 14:31 UTC (permalink / raw)
To: Vyacheslav Yurkov; +Cc: linux-kernel, Greg Kroah-Hartman
Vyacheslav,
thanks for your patch.
For now it introduces a new bug.
Vyacheslav Yurkov <uvv.mail <at> gmail.com> writes:
>
> Hi guys,
> I found an issue in managed version of dma_declare_coherent_memory,
> i.e. in dmam_declare_coherent_memory. It looks like the return value
> of dma_declare_coherent_memory is zero in case of error and a
> requested flag on success,
You should take this into account and not ignore the value of rc at all
by using flags instead, which is not being altered by
dma_declare_coherent_memory() making the latter appear to succeed
always.
> which dmam_* version doesn't take into
> account and this leads to leaking of resources.
>
> ---
> Yours sincerely,
> Vyacheslav V. Yurkov
>
> P.S.: I'm not subscribed to the maillist, so please include me in CC
> when responding to this Email.
>
> ---
> --- linux-4.5.2/drivers/base/dma-mapping.c.orig 2016-04-20
> 08:46:35.000000000 +0200
> +++ linux-4.5.2/drivers/base/dma-mapping.c 2016-04-28 10:15:34.295080491
> +0200
> <at> <at> -198,10 +198,15 <at> <at> int dmam_declare_coherent_memory(struct
>
> rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
> flags);
> - if (rc == 0)
> +
> + if ((flags & DMA_MEMORY_MAP) == DMA_MEMORY_MAP ||
> + (flags & DMA_MEMORY_IO) == DMA_MEMORY_IO) {
> devres_add(dev, res);
> - else
> + rc = 0;
> + } else {
> devres_free(res);
> + rc = -ENOMEM;
> + }
>
> return rc;
> }
>
> Signed-off-by: Vyacheslav Yurkov <uvv.mail <at> gmail.com>
>
Best Regards,
Andrey
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers/base: wrong return value of dmam_declare_coherent_memory
2016-04-28 14:31 [PATCH] drivers/base: wrong return value of dmam_declare_coherent_memory Andrey Gursky
@ 2016-04-28 15:11 ` Vyacheslav Yurkov
2016-06-14 7:58 ` [PATCH] [RESEND] drivers/base dmam_declare_coherent_memory leaks Vyacheslav V. Yurkov
0 siblings, 1 reply; 3+ messages in thread
From: Vyacheslav Yurkov @ 2016-04-28 15:11 UTC (permalink / raw)
To: Andrey Gursky; +Cc: linux-kernel, Greg Kroah-Hartman
On 28.04.2016 16:31, Andrey Gursky wrote:
> Vyacheslav,
>
> thanks for your patch.
>
> For now it introduces a new bug.
>
> Vyacheslav Yurkov <uvv.mail <at> gmail.com> writes:
>
>>
>> Hi guys,
>> I found an issue in managed version of dma_declare_coherent_memory,
>> i.e. in dmam_declare_coherent_memory. It looks like the return value
>> of dma_declare_coherent_memory is zero in case of error and a
>> requested flag on success,
>
> You should take this into account and not ignore the value of rc at all
> by using flags instead, which is not being altered by
> dma_declare_coherent_memory() making the latter appear to succeed
> always.
>
>
Hi Andrey,
Thanks for spotting this. My intention was to compare rc of course,
not the flags. While thinking it over I came up with improved version,
which is inlined below.
---
Yours sincerely,
Vyacheslav V. Yurkov
P.S.: I'm not subscribed to the maillist, so please include me in CC
when responding to this Email.
---
--- linux-4.5.2/drivers/base/dma-mapping.c.orig 2016-04-20 08:46:35.000000000 +0200
+++ linux-4.5.2/drivers/base/dma-mapping.c 2016-04-28 16:55:13.211945276 +0200
@@ -198,10 +198,14 @@ int dmam_declare_coherent_memory(struct
rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
flags);
- if (rc == 0)
+
+ if ((rc & (DMA_MEMORY_MAP | DMA_MEMORY_IO)) != 0) {
devres_add(dev, res);
- else
+ rc = 0;
+ } else {
devres_free(res);
+ rc = -ENOMEM;
+ }
return rc;
}
Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] [RESEND] drivers/base dmam_declare_coherent_memory leaks
2016-04-28 15:11 ` Vyacheslav Yurkov
@ 2016-06-14 7:58 ` Vyacheslav V. Yurkov
0 siblings, 0 replies; 3+ messages in thread
From: Vyacheslav V. Yurkov @ 2016-06-14 7:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Vyacheslav V. Yurkov
dmam_declare_coherent_memory doesn't take into account the return
value of dma_declare_coherent_memory, which leads to incorrect resource
handling
Signed-off-by: Vyacheslav V. Yurkov <uvv.mail@gmail.com>
---
drivers/base/dma-mapping.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index d799662..f5d2132 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -198,10 +198,13 @@ int dmam_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
rc = dma_declare_coherent_memory(dev, phys_addr, device_addr, size,
flags);
- if (rc == 0)
+ if (rc) {
devres_add(dev, res);
- else
+ rc = 0;
+ } else {
devres_free(res);
+ rc = -ENOMEM;
+ }
return rc;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-14 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28 14:31 [PATCH] drivers/base: wrong return value of dmam_declare_coherent_memory Andrey Gursky
2016-04-28 15:11 ` Vyacheslav Yurkov
2016-06-14 7:58 ` [PATCH] [RESEND] drivers/base dmam_declare_coherent_memory leaks Vyacheslav V. Yurkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).