netdev - Re: Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there are no longer any descriptors using it") breaks stmmac?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 Nov 2017 16:50:38 +0000
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Niklas Cassel <niklas.cassel@...s.com>
CC:     Joao Pinto <Joao.Pinto@...opsys.com>,
        linux-netdev <netdev@...r.kernel.org>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>
Subject: Re: Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there are
 no longer any descriptors using it") breaks stmmac?

Hi Niklas,

Thanks for the detailed explanation!

On 30-11-2017 03:51, Niklas Cassel wrote:
>
> Could you try to see if the following patch
> solves your problem:
> (still don't see why it should be needed,
> considering stmmac_tx_clean() should set all non-NULL
> entries to NULL)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 1763e48c84e2..1d30b20b3740 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2830,6 +2830,7 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>         tx_q->tx_skbuff_dma[first_entry].buf = des;
>         tx_q->tx_skbuff_dma[first_entry].len = skb_headlen(skb);
> +       tx_q->tx_skbuff[first_entry] = NULL;
>  
>         first->des0 = cpu_to_le32(des);
>  
> @@ -3003,6 +3004,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>         first = desc;
>  
> +       tx_q->tx_skbuff[first_entry] = NULL;
> +
>         enh_desc = priv->plat->enh_desc;
>         /* To program the descriptors according to the size of the frame */
>         if (enh_desc)

I confirm that applying 05cf0d1bf4 ("net: stmmac: free an skb
first when there are no longer any descriptors using it") and
this patch makes my setup work perfectly in multi-queue
configuration. Can you send an official patch to -net?

You can add my:
Tested-by: Jose Abreu <joabreu@...opsys.com>

Also, maybe we should try to understand why stmmac_tx_clean() is
not setting the entry to NULL?

Thanks and Best Regards,
Jose Miguel Abreu