netdev - RE: [PATCH net,stable 1/1] net: ethernet: stmmac: free tx skb buffer in 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] [day] [month] [year] [list]
Message-ID: <AM6PR0402MB3607193D973D43A2346754E0FF830@AM6PR0402MB3607.eurprd04.prod.outlook.com>
Date:   Wed, 10 Jun 2020 03:43:47 +0000
From:   Andy Duan <fugang.duan@....com>
To:     "davem@...emloft.net" <davem@...emloft.net>,
        "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>
CC:     "kuba@...nel.org" <kuba@...nel.org>,
        "mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net,stable 1/1] net: ethernet: stmmac: free tx skb buffer
 in stmmac_resume()

Giuseppe/Alexandre/Jose/David, can you give comment on the patch ?

From: Andy Duan Sent: Friday, June 5, 2020 10:06 AM
> Ping...
> 
> From: Andy Duan <fugang.duan@....com> Sent: Tuesday, June 2, 2020 3:58
> PM
> > From: Fugang Duan <fugang.duan@....com>
> >
> > When do suspend/resume test, there have WARN_ON() log dump from
> > stmmac_xmit() funciton, the code logic:
> > 	entry = tx_q->cur_tx;
> > 	first_entry = entry;
> > 	WARN_ON(tx_q->tx_skbuff[first_entry]);
> >
> > In normal case, tx_q->tx_skbuff[txq->cur_tx] should be NULL because
> > the skb should be processed and freed in stmmac_tx_clean().
> >
> > But stmmac_resume() reset queue parameters like below, skb buffers may
> > not be freed.
> > 	tx_q->cur_tx = 0;
> > 	tx_q->dirty_tx = 0;
> >
> > So free tx skb buffer in stmmac_resume() to avoid warning and memory
> leak.
> >
> > log:
> > [   46.139824] ------------[ cut here ]------------
> > [   46.144453] WARNING: CPU: 0 PID: 0 at
> > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3235
> > stmmac_xmit+0x7a0/0x9d0
> > [   46.154969] Modules linked in: crct10dif_ce vvcam(O) flexcan can_dev
> > [   46.161328] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G           O
> > 5.4.24-2.1.0+g2ad925d15481 #1
> > [   46.170369] Hardware name: NXP i.MX8MPlus EVK board (DT)
> > [   46.175677] pstate: 80000005 (Nzcv daif -PAN -UAO)
> > [   46.180465] pc : stmmac_xmit+0x7a0/0x9d0
> > [   46.184387] lr : dev_hard_start_xmit+0x94/0x158
> > [   46.188913] sp : ffff800010003cc0
> > [   46.192224] x29: ffff800010003cc0 x28: ffff000177e2a100
> > [   46.197533] x27: ffff000176ef0840 x26: ffff000176ef0090
> > [   46.202842] x25: 0000000000000000 x24: 0000000000000000
> > [   46.208151] x23: 0000000000000003 x22: ffff8000119ddd30
> > [   46.213460] x21: ffff00017636f000 x20: ffff000176ef0cc0
> > [   46.218769] x19: 0000000000000003 x18: 0000000000000000
> > [   46.224078] x17: 0000000000000000 x16: 0000000000000000
> > [   46.229386] x15: 0000000000000079 x14: 0000000000000000
> > [   46.234695] x13: 0000000000000003 x12: 0000000000000003
> > [   46.240003] x11: 0000000000000010 x10: 0000000000000010
> > [   46.245312] x9 : ffff00017002b140 x8 : 0000000000000000
> > [   46.250621] x7 : ffff00017636f000 x6 : 0000000000000010
> > [   46.255930] x5 : 0000000000000001 x4 : ffff000176ef0000
> > [   46.261238] x3 : 0000000000000003 x2 : 00000000ffffffff
> > [   46.266547] x1 : ffff000177e2a000 x0 : 0000000000000000
> > [   46.271856] Call trace:
> > [   46.274302]  stmmac_xmit+0x7a0/0x9d0
> > [   46.277874]  dev_hard_start_xmit+0x94/0x158
> > [   46.282056]  sch_direct_xmit+0x11c/0x338
> > [   46.285976]  __qdisc_run+0x118/0x5f0
> > [   46.289549]  net_tx_action+0x110/0x198
> > [   46.293297]  __do_softirq+0x120/0x23c
> > [   46.296958]  irq_exit+0xb8/0xd8
> > [   46.300098]  __handle_domain_irq+0x64/0xb8
> > [   46.304191]  gic_handle_irq+0x5c/0x148
> > [   46.307936]  el1_irq+0xb8/0x180
> > [   46.311076]  cpuidle_enter_state+0x84/0x360
> > [   46.315256]  cpuidle_enter+0x34/0x48
> > [   46.318829]  call_cpuidle+0x18/0x38
> > [   46.322314]  do_idle+0x1e0/0x280
> > [   46.325539]  cpu_startup_entry+0x24/0x40
> > [   46.329460]  rest_init+0xd4/0xe0
> > [   46.332687]  arch_call_rest_init+0xc/0x14
> > [   46.336695]  start_kernel+0x420/0x44c
> > [   46.340353] ---[ end trace bc1ee695123cbacd ]---
> >
> > Signed-off-by: Fugang Duan <fugang.duan@....com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 73677c3b33b6..3c0a2d8765f9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -1548,6 +1548,15 @@ static void dma_free_tx_skbufs(struct
> > stmmac_priv *priv, u32 queue)
> >  		stmmac_free_tx_buffer(priv, queue, i);  }
> >
> > +static void stmmac_free_tx_skbufs(struct stmmac_priv *priv) {
> > +	u32 tx_queue_cnt = priv->plat->tx_queues_to_use;
> > +	u32 queue;
> > +
> > +	for (queue = 0; queue < tx_queue_cnt; queue++)
> > +		dma_free_tx_skbufs(priv, queue);
> > +}
> > +
> >  /**
> >   * free_dma_rx_desc_resources - free RX dma desc resources
> >   * @priv: private structure
> > @@ -5186,6 +5195,7 @@ int stmmac_resume(struct device *dev)
> >
> >  	stmmac_reset_queues_param(priv);
> >
> > +	stmmac_free_tx_skbufs(priv);
> >  	stmmac_clear_descriptors(priv);
> >
> >  	stmmac_hw_setup(ndev, false);
> > --
> > 2.17.1