| 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
| ||
|
Date: Wed, 13 Oct 2021 18:13:48 +0200
From: Andreas Oetken <ennoerlangen@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
Murali Karicheri <m-karicheri2@...com>
Subject: Re: [PATCH] net: hsr: Add support for redbox supervision frames
Thank you Jakub for your notes.
Am Mittwoch, dem 13.10.2021 um 08:50 -0700 schrieb Jakub Kicinski:
> 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.
Sorry, I will not include it next time.
>
> > 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?
This code path is handling frames from the network. The supervision
frames are broadcasted by each device. What type of additional input
checking are you thinking of? I think I should check skb->len before
pulling/accessing right?
>
> > 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.
Changed it.
>
> > +} __packed;
>
> There's no need to pack structs which only have members of size 1.
>
Changed it too.
Powered by blists - more mailing lists