linux-kernel - RE: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx

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:   Tue, 5 May 2020 09:27:13 +0000
From:   "Ooi, Joyce" <joyce.ooi@...el.com>
To:     David Miller <davem@...emloft.net>
CC:     "thor.thayer@...ux.intel.com" <thor.thayer@...ux.intel.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Westergreen, Dalon" <dalon.westergreen@...el.com>,
        "Tan, Ley Foon" <ley.foon.tan@...el.com>,
        "See, Chin Liang" <chin.liang.see@...el.com>,
        "Nguyen, Dinh" <dinh.nguyen@...el.com>
Subject: RE: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores
 tx_buffer call response

> -----Original Message-----
> From: David Miller <davem@...emloft.net>
> Sent: Tuesday, May 5, 2020 1:40 AM
> To: Ooi, Joyce <joyce.ooi@...el.com>
> Cc: thor.thayer@...ux.intel.com; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; Westergreen, Dalon <dalon.westergreen@...el.com>;
> Tan, Ley Foon <ley.foon.tan@...el.com>; See, Chin Liang
> <chin.liang.see@...el.com>; Nguyen, Dinh <dinh.nguyen@...el.com>
> Subject: Re: [PATCHv2 01/10] net: eth: altera: tse_start_xmit ignores tx_buffer
> call response
> 
> From: Joyce Ooi <joyce.ooi@...el.com>
> Date: Mon,  4 May 2020 16:25:49 +0800
> 
> > The return from tx_buffer call in tse_start_xmit is inapropriately
> > ignored.  tse_buffer calls should return
> > 0 for success or NETDEV_TX_BUSY.  tse_start_xmit should return not
> > report a successful transmit when the tse_buffer call returns an error
> > condition.
> 
> From driver.txt:
> 
> ====================
> 1) The ndo_start_xmit method must not return NETDEV_TX_BUSY under
>    any normal circumstances.  It is considered a hard error unless
>    there is no way your device can tell ahead of time when it's
>    transmit function will become busy.
> ====================
> 
> The problem is that when you return this error code, something has to trigger
> restarting the transmit queue to start sending packets to your device again.  The
> usual mechanism is waking the transmit queue, but it's obviously already awake
> since your transmit routine is being called.  Therefore nothing will reliably restart
> the queue when you return this error code.
> 
> The best thing to do honestly is to drop the packet and return NETDEV_TX_OK,
> meanwhile bumping a statistic counter to record this event.

My change is similar to this hard error mentioned in drvier.txt:
/* This is a hard error log it. */
if (TX_BUFFS_AVAIL(dp) <= (skb_shinfo(skb)->nr_frags + 1)) {
	netif_stop_queue(dev);
	unlock_tx(dp);
	printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
	       dev->name);
	return NETDEV_TX_BUSY;
}

So, before returning NETDEV_TX_BUSY, I can stop the queue first by calling
netif_stop_queue().