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, 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ