netdev - Re: [PATCH net-next v2] vxlan: fix handling of the inner 8021Q tagged frame

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, 02 Apr 2014 21:10:59 +0900
From:	Atzm Watanabe <atzm@...atosphere.co.jp>
To:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
Cc:	netdev@...r.kernel.org,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net-next v2] vxlan: fix handling of the inner 8021Q tagged frame


Hi Makita-san,

At Wed, 02 Apr 2014 17:30:55 +0900,
Toshiaki Makita wrote:
> 
> (2014/04/01 23:27), Atzm Watanabe wrote:
> > Currently the implementation can forward the 8021Q tagged frame,
> > but the FDB cannot learn the VID.
> > So there is a possibility of forwarding the frame to wrong VTEP,
> > when same LLADDR exists on different VLANs.
> > 
> > This patch supports only single tagged frame, so the outermost
> > tag will be used when handling the 8021AD Q-in-Q frame.
> > 
> > v2: Fix probably unsafe operation on the struct vxlan_key.
> >     The outermost tag will be used when handling the 8021AD
> >     Q-in-Q frame.  Based on Stephen Hemminger's comments.
> > 
> > Signed-off-by: Atzm Watanabe <atzm@...atosphere.co.jp>
> ...
> > @@ -1215,8 +1257,18 @@ static void vxlan_rcv(struct vxlan_sock *vs,
> >  #endif
> >  	}
> >  
> > +	ether_addr_copy(key.eth_addr, eth_hdr(skb)->h_source);
> > +	switch (ntohs(eth_hdr(skb)->h_proto)) {
> > +	case ETH_P_8021Q:
> > +	case ETH_P_8021AD:
> > +		key.vlan_id = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI) & VLAN_VID_MASK;
> > +		break;
> 
> It seems that we can't segregate skbs tagged by same vlan id but
> different vlan protocols.

Yes, but I believe it is better than all vlan protocols are ignored.

Of course it still has problems when multiple protocols (0x8100,
0x88a8, 0x9100, ...) are mixed in a network, but I want to fix
a problem in case of single tagged, at the beginning.


> > +	default:
> > +		key.vlan_id = 0;
> > +	}
> > +
> >  	if ((vxlan->flags & VXLAN_F_LEARN) &&
> > -	    vxlan_snoop(skb->dev, &saddr, eth_hdr(skb)->h_source))
> > +	    vxlan_snoop(skb->dev, &saddr, &key))
> >  		goto drop;
> >  
> >  	skb_reset_network_header(skb);
> ...
> > @@ -1983,23 +2056,28 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
> >  #endif
> >  	}
> >  
> > -	f = vxlan_find_mac(vxlan, eth->h_dest);
> > +	ether_addr_copy(key.eth_addr, eth->h_dest);
> > +	key.vlan_id = vlan_tx_tag_present(skb) ? vlan_tx_tag_get_id(skb) : 0;
> 
> Can't we assume that skbs always have HW accelarated vlan tags?

Agreed.  Will fix.


Thank you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html