| 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
| ||
|
Message-ID: <20170405171001.GD21965@lunn.ch>
Date: Wed, 5 Apr 2017 19:10:01 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Juergen Borleis <jbe@...gutronix.de>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
f.fainelli@...il.com, kernel@...gutronix.de,
vivien.didelot@...oirfairelinux.com, davem@...emloft.net
Subject: Re: [PATCH 1/4] net: dsa: add support for the SMSC-LAN9303 tagging
format
On Wed, Apr 05, 2017 at 11:20:21AM +0200, Juergen Borleis wrote:
> To define the outgoing port and to discover the incoming port a regular
> VLAN tag is used by the LAN9303. But its VID meaning is 'special'.
>
> This tag handler/filter depends on some hardware features which must be
> enabled in the device to provide and make use of this special VLAN tag
> to control the destination and the source of an ethernet packet.
>
> Signed-off-by: Juergen Borleis <jbe@...gutronix.de>
> ---
> include/net/dsa.h | 1 +
> net/dsa/Kconfig | 3 +
> net/dsa/Makefile | 1 +
> net/dsa/dsa.c | 3 +
> net/dsa/dsa_priv.h | 3 +
> net/dsa/tag_lan9303.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 166 insertions(+)
> create mode 100644 net/dsa/tag_lan9303.c
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 4e13e695f0251..4fb1f2b086b05 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -31,6 +31,7 @@ enum dsa_tag_protocol {
> DSA_TAG_PROTO_EDSA,
> DSA_TAG_PROTO_BRCM,
> DSA_TAG_PROTO_QCA,
> + DSA_TAG_PROTO_LAN9303,
> DSA_TAG_LAST, /* MUST BE LAST */
> };
>
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 9649238eef404..22c8bd69ff71c 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -31,4 +31,7 @@ config NET_DSA_TAG_TRAILER
> config NET_DSA_TAG_QCA
> bool
>
> +config NET_DSA_TAG_LAN9303
> + bool
> +
> endif
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 31d343796251d..aafc74f2cb193 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -8,3 +8,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
> dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
> dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
> dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
> +dsa_core-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index b6d4f6a23f06c..f93f78de23af3 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -53,6 +53,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
> #ifdef CONFIG_NET_DSA_TAG_QCA
> [DSA_TAG_PROTO_QCA] = &qca_netdev_ops,
> #endif
> +#ifdef CONFIG_NET_DSA_TAG_LAN9303
> + [DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
> +#endif
> [DSA_TAG_PROTO_NONE] = &none_ops,
> };
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 0706a511244e9..a54cfc8aefa83 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -85,4 +85,7 @@ extern const struct dsa_device_ops brcm_netdev_ops;
> /* tag_qca.c */
> extern const struct dsa_device_ops qca_netdev_ops;
>
> +/* tag_lan9303.c */
> +extern const struct dsa_device_ops lan9303_netdev_ops;
> +
> #endif
> diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
> new file mode 100644
> index 0000000000000..ad04c6d447f77
> --- /dev/null
> +++ b/net/dsa/tag_lan9303.c
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (C) 2017 Pengutronix, Juergen Borleis <jbe@...gutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/etherdevice.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include "dsa_priv.h"
> +
> +/* To define the outgoing port and to discover the incoming port a regular
> + * VLAN tag is used by the LAN9303. But its VID meaning is 'special':
> + *
> + * Dest MAC Src MAC TAG Type
> + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 | 1 2 |...
> + * |<------->|
> + * TAG:
> + * |<------------->|
> + * | 1 2 | 3 4 |
> + * TPID VID
> + * 0x8100
> + *
> + * VID bit 3 indicates a request for an ALR lookup.
> + *
> + * If VID bit 3 is zero, then bits 0 and 1 specify the destination port
> + * (0, 1, 2) or broadcast (3) or the source port (1, 2).
> + *
> + * VID bit 4 is used to specify if the STP port state should be overridden.
> + * Required when no forwarding between the external ports should happen.
> + */
Hi Juergen
Nice comment, thanks.
> +
> +#define LAN9303_TAG_LEN 4
> +
> +static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
...
> + /* make room between MACs and Ether-Type */
> + memmove(skb->data, skb->data + LAN9303_TAG_LEN, 2 * ETH_ALEN);
> +
> + lan9303_tag = (u16 *)(skb->data + 2 * ETH_ALEN);
> + lan9303_tag[0] = htons(ETH_P_8021Q);
> + lan9303_tag[1] = htons(p->dp->index | BIT(4));
> +static int lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
> + struct packet_type *pt, struct net_device *orig_dev)
> +{
> + u16 *lan9303_tag;
> + struct dsa_switch_tree *dst = dev->dsa_ptr;
> + struct dsa_switch *ds = dst->ds[0];
> + unsigned int source_port;
> +
> + if (unlikely(!dst)) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet, due to missing switch tree device\n");
> + goto out_drop;
> + }
By the time you get here, you have already dereferenced dst, in order
to get ds. So this is pointless.
> + lan9303_tag = (u16 *)(skb->data - 2);
> +
> + if (lan9303_tag[0] != htons(0x8100)) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid VLAN marker\n");
> + goto out_drop;
> + }
In the transmit function, you use ETH_P_8021Q, please do so here as
well.
> + source_port = ntohs(lan9303_tag[1]) & 0x3;
> +
> + if (source_port >= DSA_MAX_PORTS) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid source port\n");
> + goto out_drop;
> + }
You can be more specific here. For this hardware, > 3 is invalid and
should be dropped. If we later add other chips which have more ports,
we can relax this check then.
> + /* remove the special VLAN tag between the MAC addresses
> + * and the current ethertype field.
> + */
> + skb_pull_rcsum(skb, 2 + 2);
> + memmove(skb->data - ETH_HLEN, skb->data - (ETH_HLEN + LAN9303_TAG_LEN),
> + 2 * ETH_ALEN);
> + skb_push(skb, ETH_HLEN);
Do you need to do anything with the checksum here? Other tagging
protocols do.
Andrew
Powered by blists - more mailing lists