netdev - Re: [PATCH net-next 0/5] Simplifications for Broadcom and Realtek DSA taggers

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:   Wed, 11 May 2022 18:22:41 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 0/5] Simplifications for Broadcom and Realtek
 DSA taggers

On Wed, May 11, 2022 at 06:14:26PM +0300, Vladimir Oltean wrote:
> This series contains the removal of some "if" conditions from the TX hot
> path in tag_brcm and tag_rtl4_a. This is made possible by the fact that
> the DSA core has previously checked that memory allocations are not
> necessary, so there is nothing that can fail.
> 
> Vladimir Oltean (5):
>   net: dsa: tag_rtl4_a: __skb_put_padto() can never fail
>   net: dsa: tag_brcm: do not account for tag length twice when padding
>   net: dsa: tag_brcm: __skb_put_padto() can never fail
>   net: dsa: tag_brcm: eliminate conditional based on offset from
>     brcm_tag_xmit_ll
>   net: dsa: tag_brcm: use dsa_etype_header_pos_tx for legacy tag
> 
>  net/dsa/tag_brcm.c   | 51 +++++++++++++++++++-------------------------
>  net/dsa/tag_rtl4_a.c |  4 +---
>  2 files changed, 23 insertions(+), 32 deletions(-)
> 
> -- 
> 2.25.1
> 

Oh, wait, the premise behind patches 1 and 3 is invalid.
dsa_realloc_skb() only ensures there are ETH_ZLEN bytes of space in the
skb for tail taggers.

	if (unlikely(needed_tailroom && skb->len < ETH_ZLEN))
		needed_tailroom += ETH_ZLEN - skb->len;

Please treat the patch set as RFC, if we have agreement on the overall
idea I can update dsa_realloc_skb() by removing "needed_tailroom && "
from the check so the guarantee applies to all taggers.