| 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: <20211013085014.4beb11e4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date: Wed, 13 Oct 2021 08:50:14 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Andreas Oetken <ennoerlangen@...il.com>
Cc: "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
Murali Karicheri <m-karicheri2@...com>,
Andreas Oetken <andreas.oetken@...mens-energy.com>
Subject: Re: [PATCH] net: hsr: Add support for redbox supervision frames
On Wed, 13 Oct 2021 09:29:51 +0200 Andreas Oetken wrote:
> added support for the redbox supervision frames
> as defined in the IEC-62439-3:2018.
>
> Signed-off-by: Andreas Oetken <andreas.oetken@...mens-energy.com>
This does not apply to netdev/net-next.
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index fdd9c00082a8..b1677b0a9202 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -511,9 +511,9 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
> }
> spin_unlock_irqrestore(&hsr->seqnr_lock, irqflags);
>
> - hsr_stag->HSR_TLV_type = type;
> + hsr_stag->tlv.HSR_TLV_type = type;
> /* TODO: Why 12 in HSRv0? */
> - hsr_stag->HSR_TLV_length = hsr->prot_version ?
> + hsr_stag->tlv.HSR_TLV_length = hsr->prot_version ?
> sizeof(struct hsr_sup_payload) : 12;
>
> /* Payload: MacAddressA */
> @@ -560,8 +560,8 @@ static void send_prp_supervision_frame(struct hsr_port *master,
> spin_lock_irqsave(&master->hsr->seqnr_lock, irqflags);
> hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
> hsr->sup_sequence_nr++;
> - hsr_stag->HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> - hsr_stag->HSR_TLV_length = sizeof(struct hsr_sup_payload);
> + hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> + hsr_stag->tlv.HSR_TLV_length = sizeof(struct hsr_sup_payload);
>
> /* Payload: MacAddressA */
> hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index d4d434b9f598..312d6a86c124 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -95,13 +95,13 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
> &((struct hsrv0_ethhdr_vlan_sp *)eth_hdr)->hsr_sup;
> }
>
> - if (hsr_sup_tag->HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> - hsr_sup_tag->HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> + if (hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> + hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> return false;
> - if (hsr_sup_tag->HSR_TLV_length != 12 &&
> - hsr_sup_tag->HSR_TLV_length != sizeof(struct hsr_sup_payload))
> + if (hsr_sup_tag->tlv.HSR_TLV_length != 12 &&
> + hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
> return false;
>
> return true;
> diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
> index bb1351c38397..e7c6efbc41af 100644
> --- a/net/hsr/hsr_framereg.c
> +++ b/net/hsr/hsr_framereg.c
> @@ -265,6 +265,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
> struct hsr_port *port_rcv = frame->port_rcv;
> struct hsr_priv *hsr = port_rcv->hsr;
> struct hsr_sup_payload *hsr_sp;
> + struct hsr_sup_tlv *hsr_sup_tlv;
> struct hsr_node *node_real;
> struct sk_buff *skb = NULL;
> struct list_head *node_db;
> @@ -312,6 +313,40 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
> /* Node has already been merged */
> goto done;
>
> + /* Leave the first HSR sup payload. */
> + skb_pull(skb, sizeof(struct hsr_sup_payload));
> +
> + /* Get second supervision tlv */
> + hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
> + /* And check if it is a redbox mac TLV */
> + if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> + /* We could stop here after pushing hsr_sup_payload,
> + * or proceed and allow macaddress_B and for redboxes.
> + */
> + /* Sanity check length */
> + if (hsr_sup_tlv->HSR_TLV_length != 6) {
> + skb_push(skb, sizeof(struct hsr_sup_payload));
> + goto done;
> + }
> + /* Leave the second HSR sup tlv. */
> + skb_pull(skb, sizeof(struct hsr_sup_tlv));
> +
> + /* Get redbox mac address. */
> + hsr_sp = (struct hsr_sup_payload *)skb->data;
> +
> + /* Check if redbox mac and node mac are equal. */
> + if (!ether_addr_equal(node_real->macaddress_A, hsr_sp->macaddress_A)) {
> + /* This is a redbox supervision frame for a VDAN! */
> + /* Push second TLV and payload here */
> + skb_push(skb, sizeof(struct hsr_sup_payload) + sizeof(struct hsr_sup_tlv));
> + goto done;
> + }
> + /* Push second TLV here */
> + skb_push(skb, sizeof(struct hsr_sup_tlv));
> + }
> + /* Push payload here */
> + skb_push(skb, sizeof(struct hsr_sup_payload));
Is this code path handling frames from the network or user space?
Does it need input checking?
> ether_addr_copy(node_real->macaddress_B, ethhdr->h_source);
> for (i = 0; i < HSR_PT_PORTS; i++) {
> if (!node_curr->time_in_stale[i] &&
> diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
> index bbaef001d55d..fc3bed792ba7 100644
> --- a/net/hsr/hsr_main.h
> +++ b/net/hsr/hsr_main.h
> @@ -43,6 +43,8 @@
> #define PRP_TLV_LIFE_CHECK_DD 20
> /* PRP V1 life check for Duplicate Accept */
> #define PRP_TLV_LIFE_CHECK_DA 21
> +/* PRP V1 life redundancy box MAC address */
> +#define PRP_TLV_REDBOX_MAC 30
>
> /* HSR Tag.
> * As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
> @@ -95,14 +97,18 @@ struct hsr_vlan_ethhdr {
> struct hsr_tag hsr_tag;
> } __packed;
>
> +struct hsr_sup_tlv {
> + __u8 HSR_TLV_type;
> + __u8 HSR_TLV_length;
u8, __u8 is for uAPI headers, which this is not.
> +} __packed;
There's no need to pack structs which only have members of size 1.
Powered by blists - more mailing lists