1. Patchew
  2. linux
  3. [PATCH net v2 0/2] Fix dma leaking
  4. View patch

[PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak

Jonas Suhr Christensen posted 2 patches 2 years, 7 months ago
[PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Jonas Suhr Christensen 2 years, 7 months ago 
Add missing conversion of address when unmapping dma region causing
unmapping to silently fail. At some point resulting in buffer
overrun eg. when releasing device.

Fixes: fdd7454ecb29 ("net: ll_temac: Fix support for little-endian platforms")

Signed-off-by: Jonas Suhr Christensen <jsc@umbraculum.org>
---
 drivers/net/ethernet/xilinx/ll_temac_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 1066420d6a83..74423adbe50d 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -299,6 +299,7 @@ static int temac_dcr_setup(struct temac_local *lp, struct platform_device *op,
 static void temac_dma_bd_release(struct net_device *ndev)
 {
 	struct temac_local *lp = netdev_priv(ndev);
+	struct cdmac_bd *bd;
 	int i;
 
 	/* Reset Local Link (DMA) */
@@ -307,9 +308,14 @@ static void temac_dma_bd_release(struct net_device *ndev)
 	for (i = 0; i < lp->rx_bd_num; i++) {
 		if (!lp->rx_skb[i])
 			break;
-		dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
+
+		bd = &lp->rx_bd_v[i];
+		dma_unmap_single(ndev->dev.parent, be32_to_cpu(bd->phys),
 				 XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
+		bd->phys = 0;
+		bd->len = 0;
 		dev_kfree_skb(lp->rx_skb[i]);
+		lp->rx_skb[i] = NULL;
 	}
 	if (lp->rx_bd_v)
 		dma_free_coherent(ndev->dev.parent,
-- 
2.39.1
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Paolo Abeni 2 years, 7 months ago 
Hi,

On Sun, 2023-02-05 at 21:11 +0100, Jonas Suhr Christensen wrote:
> Add missing conversion of address when unmapping dma region causing
> unmapping to silently fail. At some point resulting in buffer
> overrun eg. when releasing device.
> 
> Fixes: fdd7454ecb29 ("net: ll_temac: Fix support for little-endian platforms")
> 
> Signed-off-by: Jonas Suhr Christensen <jsc@umbraculum.org>

I'm sorry for nit-picking, but you must avoid empty lines in the tag
area. Please post a v2 avoiding the empty line between the Fixes and
sob tags (both here and in the next patch).

You can retain (include in the tag area) the already collected
reviewed-by/acked-by tags.

Thanks,

Paolo
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Paolo Abeni 2 years, 7 months ago 
On Tue, 2023-02-07 at 12:27 +0100, Paolo Abeni wrote:
> Hi,
> 
> On Sun, 2023-02-05 at 21:11 +0100, Jonas Suhr Christensen wrote:
> > Add missing conversion of address when unmapping dma region causing
> > unmapping to silently fail. At some point resulting in buffer
> > overrun eg. when releasing device.
> > 
> > Fixes: fdd7454ecb29 ("net: ll_temac: Fix support for little-endian platforms")
> > 
> > Signed-off-by: Jonas Suhr Christensen <jsc@umbraculum.org>
> 
> I'm sorry for nit-picking, but you must avoid empty lines in the tag
> area. Please post a v2 avoiding the empty line between the Fixes and
> sob tags (both here and in the next patch).
> 
> You can retain (include in the tag area) the already collected
> reviewed-by/acked-by tags.

I'm sorry,  I'm low on coffee. I forgot to mention a more relevant
thing:

> @@ -307,9 +308,14 @@  static void temac_dma_bd_release(struct net_device *ndev)
> 	for (i = 0; i < lp->rx_bd_num; i++) {
> 		if (!lp->rx_skb[i])
> 			break;
> -		dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
> +
> +		bd = &lp->rx_bd_v[i];
> +		dma_unmap_single(ndev->dev.parent, be32_to_cpu(bd->phys),

The above be32_to_cpu() introduces a new build warning. as phys type is
u32. It looks like the existing code generates a lot of similar warns.

You can either try change to phys type to __be32 (likely not suitable
for -net and possibly can introduce even more warnings elsewhere) or
explicitly cast the argument.

Thanks,

Paolo
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Jakub Kicinski 2 years, 7 months ago 
On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:
> You can either try change to phys type to __be32 (likely not suitable
> for -net and possibly can introduce even more warnings elsewhere)

FWIW that seems like the best option to me as well. Let's ignore the
sparse warning for v3 and try to switch phys to __be32 in a separate
patch for net-next. No point adding force casts just to have to remove
them a week later, given how prevalent the problem is.

> or explicitly cast the argument.
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Jonas Suhr Christensen 2 years, 6 months ago 
On Tue, Feb 7, 2023, at 19:42, Jakub Kicinski wrote:
> On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:
>> You can either try change to phys type to __be32 (likely not suitable
>> for -net and possibly can introduce even more warnings elsewhere)
>
> FWIW that seems like the best option to me as well. Let's ignore the
> sparse warning for v3 and try to switch phys to __be32 in a separate
> patch for net-next. No point adding force casts just to have to remove
> them a week later, given how prevalent the problem is.
>
>> or explicitly cast the argument.

I no longer have access to the hardware, so I'm not rewriting the batch. Feel free to take ownership of it and fix what's needed.
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Jakub Kicinski 2 years, 6 months ago 
On Mon, 13 Mar 2023 19:37:00 +0100 Jonas Suhr Christensen wrote:
> On Tue, Feb 7, 2023, at 19:42, Jakub Kicinski wrote:
> > On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:  
> >> You can either try change to phys type to __be32 (likely not suitable
> >> for -net and possibly can introduce even more warnings elsewhere)  
> >
> > FWIW that seems like the best option to me as well. Let's ignore the
> > sparse warning for v3 and try to switch phys to __be32 in a separate
> > patch for net-next. No point adding force casts just to have to remove
> > them a week later, given how prevalent the problem is.
> >  
> >> or explicitly cast the argument.  
> 
> I no longer have access to the hardware, so I'm not rewriting the
> batch. Feel free to take ownership of it and fix what's needed.

Ack.

Harini, are you the designated maintainer for this driver? Could you
add a MAINTAINERS entry for it? I don't see one right now.
And possibly pick up these patches / fix the problem, if you have 
the cycles?
RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Katakam, Harini 2 years, 6 months ago 
Hi Jakub, Jonas,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, March 14, 2023 12:19 AM
> To: Jonas Suhr Christensen <jsc@umbraculum.org>; Katakam, Harini
> <harini.katakam@amd.com>
> Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
> <xuhaoyue1@hisilicon.com>; huangjunxian <huangjunxian6@hisilicon.com>;
> Wang Qing <wangqing@vivo.com>; Yang Yingliang
> <yangyingliang@huawei.com>; Esben Haabendal <esben@geanix.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
> 
> On Mon, 13 Mar 2023 19:37:00 +0100 Jonas Suhr Christensen wrote:
> > On Tue, Feb 7, 2023, at 19:42, Jakub Kicinski wrote:
> > > On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:
> > >> You can either try change to phys type to __be32 (likely not
> > >> suitable for -net and possibly can introduce even more warnings
> > >> elsewhere)
> > >
> > > FWIW that seems like the best option to me as well. Let's ignore the
> > > sparse warning for v3 and try to switch phys to __be32 in a separate
> > > patch for net-next. No point adding force casts just to have to
> > > remove them a week later, given how prevalent the problem is.
> > >
> > >> or explicitly cast the argument.
> >
> > I no longer have access to the hardware, so I'm not rewriting the
> > batch. Feel free to take ownership of it and fix what's needed.
> 
> Ack.
> 
> Harini, are you the designated maintainer for this driver? Could you add a
> MAINTAINERS entry for it? I don't see one right now.
> And possibly pick up these patches / fix the problem, if you have the cycles?

Sure, Srinivas (cced) will pick up this series and send a v3.
I'll get back on the state of this IP/driver for the maintainers list. Will include
that patch in the beginning of the series as well.

Regards,
Harini
Re: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Christophe JAILLET 2 years, 2 months ago 
Le 14/03/2023 à 06:15, Katakam, Harini a écrit :
> Hi Jakub, Jonas,
> 
>> -----Original Message-----
>> From: Jakub Kicinski <kuba@kernel.org>
>> Sent: Tuesday, March 14, 2023 12:19 AM
>> To: Jonas Suhr Christensen <jsc@umbraculum.org>; Katakam, Harini
>> <harini.katakam@amd.com>
>> Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
>> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
>> Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
>> <xuhaoyue1@hisilicon.com>; huangjunxian <huangjunxian6@hisilicon.com>;
>> Wang Qing <wangqing@vivo.com>; Yang Yingliang
>> <yangyingliang@huawei.com>; Esben Haabendal <esben@geanix.com>;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
>>
>> On Mon, 13 Mar 2023 19:37:00 +0100 Jonas Suhr Christensen wrote:
>>> On Tue, Feb 7, 2023, at 19:42, Jakub Kicinski wrote:
>>>> On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:
>>>>> You can either try change to phys type to __be32 (likely not
>>>>> suitable for -net and possibly can introduce even more warnings
>>>>> elsewhere)
>>>>
>>>> FWIW that seems like the best option to me as well. Let's ignore the
>>>> sparse warning for v3 and try to switch phys to __be32 in a separate
>>>> patch for net-next. No point adding force casts just to have to
>>>> remove them a week later, given how prevalent the problem is.
>>>>
>>>>> or explicitly cast the argument.
>>>
>>> I no longer have access to the hardware, so I'm not rewriting the
>>> batch. Feel free to take ownership of it and fix what's needed.
>>
>> Ack.
>>
>> Harini, are you the designated maintainer for this driver? Could you add a
>> MAINTAINERS entry for it? I don't see one right now.
>> And possibly pick up these patches / fix the problem, if you have the cycles?
> 
> Sure, Srinivas (cced) will pick up this series and send a v3.
> I'll get back on the state of this IP/driver for the maintainers list. Will include
> that patch in the beginning of the series as well.
> 
> Regards,
> Harini
> 

Hi,

this patch, or an updated version, has not reached -next yet.

Does someone still working on it, or did it got lost?

CJ
RE: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Katakam, Harini 2 years, 2 months ago 
Hi Christophe,

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Sent: Saturday, July 8, 2023 6:46 PM
> To: Katakam, Harini <harini.katakam@amd.com>; Jakub Kicinski
> <kuba@kernel.org>; Jonas Suhr Christensen <jsc@umbraculum.org>
> Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
> <xuhaoyue1@hisilicon.com>; huangjunxian <huangjunxian6@hisilicon.com>;
> Wang Qing <wangqing@vivo.com>; Yang Yingliang
> <yangyingliang@huawei.com>; Esben Haabendal <esben@geanix.com>; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Neeli, Srinivas
> <srinivas.neeli@amd.com>
> Subject: Re: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
> 
> Le 14/03/2023 à 06:15, Katakam, Harini a écrit :
> > Hi Jakub, Jonas,
> >
> >> -----Original Message-----
> >> From: Jakub Kicinski <kuba@kernel.org>
> >> Sent: Tuesday, March 14, 2023 12:19 AM
> >> To: Jonas Suhr Christensen <jsc@umbraculum.org>; Katakam, Harini
> >> <harini.katakam@amd.com>
> >> Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
> >> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> >> Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
> >> <xuhaoyue1@hisilicon.com>; huangjunxian
> >> <huangjunxian6@hisilicon.com>; Wang Qing <wangqing@vivo.com>; Yang
> >> Yingliang <yangyingliang@huawei.com>; Esben Haabendal
> >> <esben@geanix.com>; linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
> >>
> >> On Mon, 13 Mar 2023 19:37:00 +0100 Jonas Suhr Christensen wrote:
> >>> On Tue, Feb 7, 2023, at 19:42, Jakub Kicinski wrote:
> >>>> On Tue, 07 Feb 2023 12:36:11 +0100 Paolo Abeni wrote:
> >>>>> You can either try change to phys type to __be32 (likely not
> >>>>> suitable for -net and possibly can introduce even more warnings
> >>>>> elsewhere)
> >>>>
> >>>> FWIW that seems like the best option to me as well. Let's ignore
> >>>> the sparse warning for v3 and try to switch phys to __be32 in a
> >>>> separate patch for net-next. No point adding force casts just to
> >>>> have to remove them a week later, given how prevalent the problem is.
> >>>>
> >>>>> or explicitly cast the argument.
> >>>
> >>> I no longer have access to the hardware, so I'm not rewriting the
> >>> batch. Feel free to take ownership of it and fix what's needed.
> >>
> >> Ack.
> >>
> >> Harini, are you the designated maintainer for this driver? Could you
> >> add a MAINTAINERS entry for it? I don't see one right now.
> >> And possibly pick up these patches / fix the problem, if you have the cycles?
> >
> > Sure, Srinivas (cced) will pick up this series and send a v3.
> > I'll get back on the state of this IP/driver for the maintainers list.
> > Will include that patch in the beginning of the series as well.
> >
> > Regards,
> > Harini
> >
> 
> Hi,
> 
> this patch, or an updated version, has not reached -next yet.
> 
> Does someone still working on it, or did it got lost?

Apologies, we dint get a chance to close on this. We'll fix and re-spin next month.
This IP and driver are in minimal support mode right now.
We'll update the maintainers entry.

Regards,
Harini
RE: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Katakam, Harini 1 year, 11 months ago 
Hi,

> -----Original Message-----
> From: Katakam, Harini <harini.katakam@amd.com>
> Sent: Monday, July 10, 2023 11:21 AM
> To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>; Jakub Kicinski
> <kuba@kernel.org>; Jonas Suhr Christensen <jsc@umbraculum.org>
> Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
> <xuhaoyue1@hisilicon.com>; huangjunxian <huangjunxian6@hisilicon.com>;
> Wang Qing <wangqing@vivo.com>; Yang Yingliang
> <yangyingliang@huawei.com>; Esben Haabendal <esben@geanix.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Neeli,
> Srinivas <srinivas.neeli@amd.com>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>
> Subject: RE: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
> 
> Hi Christophe,
> 
> > -----Original Message-----
> > From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > Sent: Saturday, July 8, 2023 6:46 PM
> > To: Katakam, Harini <harini.katakam@amd.com>; Jakub Kicinski
> > <kuba@kernel.org>; Jonas Suhr Christensen <jsc@umbraculum.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; David S.
> > Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>;
> > Michal Simek <michal.simek@xilinx.com>; Haoyue Xu
> > <xuhaoyue1@hisilicon.com>; huangjunxian
> <huangjunxian6@hisilicon.com>;
> > Wang Qing <wangqing@vivo.com>; Yang Yingliang
> > <yangyingliang@huawei.com>; Esben Haabendal <esben@geanix.com>;
> linux-
> > arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Neeli,
> > Srinivas <srinivas.neeli@amd.com>
> > Subject: Re: RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources
> > leak
> >
<snip>
> > >>
> > >> Harini, are you the designated maintainer for this driver? Could
> > >> you add a MAINTAINERS entry for it? I don't see one right now.
> > >> And possibly pick up these patches / fix the problem, if you have the
> cycles?
> > >
> > > Sure, Srinivas (cced) will pick up this series and send a v3.
> > > I'll get back on the state of this IP/driver for the maintainers list.
> > > Will include that patch in the beginning of the series as well.
> > >
> > > Regards,
> > > Harini
> > >
> >
> > Hi,
> >
> > this patch, or an updated version, has not reached -next yet.
> >
> > Does someone still working on it, or did it got lost?
> 
> Apologies, we dint get a chance to close on this. We'll fix and re-spin next
> month.
> This IP and driver are in minimal support mode right now.
> We'll update the maintainers entry.

We checked internally for hardware to test this and concluded that this IP
is no longer supported and driver cannot be maintained.
I have just sent a patch to add an obsolete MAINTAINERS entry for this driver.
Apologies for not updating the state earlier.

Regards,
Harini
Re: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by esben@geanix.com 2 years, 7 months ago 
Jonas Suhr Christensen <jsc@umbraculum.org> writes:

> Add missing conversion of address when unmapping dma region causing
> unmapping to silently fail. At some point resulting in buffer
> overrun eg. when releasing device.
>
> Fixes: fdd7454ecb29 ("net: ll_temac: Fix support for little-endian platforms")
>
> Signed-off-by: Jonas Suhr Christensen <jsc@umbraculum.org>

Acked-by: Esben Haabendal <esben@geanix.com>
RE: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
Posted by Katakam, Harini 2 years, 7 months ago 
> -----Original Message-----
> From: Jonas Suhr Christensen <jsc@umbraculum.org>
> Sent: Monday, February 6, 2023 1:41 AM
> To: netdev@vger.kernel.org
> Cc: jsc@umbraculum.org; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Michal Simek
> <michal.simek@xilinx.com>; Katakam, Harini <harini.katakam@amd.com>;
> Haoyue Xu <xuhaoyue1@hisilicon.com>; huangjunxian
> <huangjunxian6@hisilicon.com>; Wang Qing <wangqing@vivo.com>; Yang
> Yingliang <yangyingliang@huawei.com>; Esben Haabendal
> <esben@geanix.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH net v2 1/2] net: ll_temac: Fix DMA resources leak
> 
> Add missing conversion of address when unmapping dma region causing
> unmapping to silently fail. At some point resulting in buffer
> overrun eg. when releasing device.
> 
> Fixes: fdd7454ecb29 ("net: ll_temac: Fix support for little-endian platforms")
> 
> Signed-off-by: Jonas Suhr Christensen <jsc@umbraculum.org>

Thanks for the patch.
Reviewed-by: Harini Katakam <harini.katakam@amd.com>

Regards,
Harini

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.