Linux Ethernet Bridging - Jan 2019 - [Bridge] [PATCH net v4] net: bridge: Fix ethernet header pointer before check skb forwardable

From: Yunjian Wang <wangyunjian at huawei.com>

The skb header should be set to ethernet header before using
is_skb_forwardable. Because the ethernet header length has been
considered in is_skb_forwardable(including dev->hard_header_len
length).

To reproduce the issue:
1, add 2 ports on linux bridge br using following commands:
$ brctl addbr br
$ brctl addif br eth0
$ brctl addif br eth1
2, the MTU of eth0 and eth1 is 1500
3, send a packet(Data 1480, UDP 8, IP 20, Ethernet 14, VLAN 4)
from eth0 to eth1

So the expect result is packet larger than 1500 cannot pass through
eth0 and eth1. But currently, the packet passes through success, it
means eth1's MTU limit doesn't take effect.

Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward path")
Cc: bridge at lists.linux-foundation.org
Cc: Nkolay Aleksandrov <nikolay at cumulusnetworks.com>
Cc: Roopa Prabhu <roopa at cumulusnetworks.com>
Cc: Stephen Hemminger <stephen at networkplumber.org>
Signed-off-by: Yunjian Wang <wangyunjian at huawei.com>
---
v2:
 -fix commit log and cc the bridge maintainers with fixes tags
v3:
 -add detailed description of packet len and construction
v4:
 -fix coding style issues
---
 net/bridge/br_forward.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 5372e20..d0f2669 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -36,10 +36,10 @@ static inline int should_deliver(const struct
net_bridge_port *p,
 
 int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct sk_buff
*skb)
 {
+	skb_push(skb, ETH_HLEN);
 	if (!is_skb_forwardable(skb->dev, skb))
 		goto drop;
 
-	skb_push(skb, ETH_HLEN);
 	br_drop_fake_rtable(skb);
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL &&
@@ -97,12 +97,11 @@ static void __br_forward(const struct net_bridge_port *to,
 		net = dev_net(indev);
 	} else {
 		if (unlikely(netpoll_tx_running(to->br->dev))) {
-			if (!is_skb_forwardable(skb->dev, skb)) {
+			skb_push(skb, ETH_HLEN);
+			if (!is_skb_forwardable(skb->dev, skb))
 				kfree_skb(skb);
-			} else {
-				skb_push(skb, ETH_HLEN);
+			else
 				br_netpoll_send_skb(to, skb);
-			}
 			return;
 		}
 		br_hook = NF_BR_LOCAL_OUT;
-- 
1.8.3.1

From: wangyunjian <wangyunjian at huawei.com>
Date: Thu, 17 Jan 2019 09:46:41 +0800
> From: Yunjian Wang <wangyunjian at huawei.com>
> 
> The skb header should be set to ethernet header before using
> is_skb_forwardable. Because the ethernet header length has been
> considered in is_skb_forwardable(including dev->hard_header_len
> length).
> 
> To reproduce the issue:
> 1, add 2 ports on linux bridge br using following commands:
> $ brctl addbr br
> $ brctl addif br eth0
> $ brctl addif br eth1
> 2, the MTU of eth0 and eth1 is 1500
> 3, send a packet(Data 1480, UDP 8, IP 20, Ethernet 14, VLAN 4)
> from eth0 to eth1
> 
> So the expect result is packet larger than 1500 cannot pass through
> eth0 and eth1. But currently, the packet passes through success, it
> means eth1's MTU limit doesn't take effect.
> 
> Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward
path")
> Cc: bridge at lists.linux-foundation.org
> Cc: Nkolay Aleksandrov <nikolay at cumulusnetworks.com>
> Cc: Roopa Prabhu <roopa at cumulusnetworks.com>
> Cc: Stephen Hemminger <stephen at networkplumber.org>
> Signed-off-by: Yunjian Wang <wangyunjian at huawei.com>
Applied and queued up for -stable.