netdev - Re: [net-next PATCH 1/2 v5] net: dsa: tag_rtl4

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, 7 Jul 2020 14:50:03 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>
Cc:     DENG Qingfang <dqfext@...il.com>,
        Mauri Sandberg <sandberg@...lfence.com>
Subject: Re: [net-next PATCH 1/2 v5] net: dsa: tag_rtl4_a: Implement Realtek 4
 byte A tag



On 7/7/2020 2:16 PM, Linus Walleij wrote:
> This implements the known parts of the Realtek 4 byte
> tag protocol version 0xA, as found in the RTL8366RB
> DSA switch.
> 
> It is designated as protocol version 0xA as a
> different Realtek 4 byte tag format with protocol
> version 0x9 is known to exist in the Realtek RTL8306
> chips.
> 
> The tag and switch chip lacks public documentation, so
> the tag format has been reverse-engineered from
> packet dumps. As only ingress traffic has been available
> for analysis an egress tag has not been possible to
> develop (even using educated guesses about bit fields)
> so this is as far as it gets. It is not known if the
> switch even supports egress tagging.
> 
> Excessive attempts to figure out the egress tag format
> was made. When nothing else worked, I just tried all bit
> combinations with 0xannp where a is protocol and p is
> port. I looped through all values several times trying
> to get a response from ping, without any positive
> result.
> 
> Using just these ingress tags however, the switch
> functionality is vastly improved and the packets find
> their way into the destination port without any
> tricky VLAN configuration. On the D-Link DIR-685 the
> LAN ports now come up and respond to ping without
> any command line configuration so this is a real
> improvement for users.
> 
> Egress packets need to be restricted to the proper
> target ports using VLAN, which the RTL8366RB DSA
> switch driver already sets up.
> 
> Cc: DENG Qingfang <dqfext@...il.com>
> Cc: Mauri Sandberg <sandberg@...lfence.com>
> Reviewed-by: Andrew Lunn <andrew@...n.ch>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>

[snip]

> + * -------------------------------------------------
> + * | MAC DA | MAC SA | 0x8899 | 2 bytes tag | Type |
> + * -------------------------------------------------
> + *
> + * The 2 bytes tag form a 16 bit big endian word. The exact
> + * meaning has been guess from packet dumps from ingress

s/guess/guessed/

[snip]

> +	port = protport & 0xff;
> +
> +	/* Remove RTL4 tag and recalculate checksum */
> +	skb_pull_rcsum(skb, RTL4_A_HDR_LEN);
> +
> +	/* Move ethernet DA and SA in front of the data */
> +	memmove(skb->data - ETH_HLEN,
> +		skb->data - ETH_HLEN - RTL4_A_HDR_LEN,
> +		2 * ETH_ALEN);
> +
> +	skb->dev = dsa_master_find_slave(dev, 0, port);

You would want to do this prior to doing the skb_pull_rcsum() and
memmove(), if the port is invalid, no point in pulling the SKB.

With those fixed:

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian