Patchwork [v7,net-next,03/12] bridge: Verify that a vlan is allowed to egress on give port

login
register
mail settings
Submitter Vlad Yasevich
Date Jan. 31, 2013, 3:12 a.m.
Message ID <1359601979-14942-4-git-send-email-vyasevic@redhat.com>
Download mbox | patch
Permalink /patch/217073/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Vlad Yasevich - Jan. 31, 2013, 3:12 a.m.
When bridge forwards a frame, make sure that a frame is allowed
to egress on that port.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/bridge/br_forward.c |    1 +
 net/bridge/br_input.c   |   10 ++++++++++
 net/bridge/br_private.h |   16 ++++++++++++++--
 net/bridge/br_vlan.c    |   20 ++++++++++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)
=?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= - Jan. 31, 2013, 8:03 p.m.
2013/1/31 Vlad Yasevich <vyasevic@redhat.com>:
> When bridge forwards a frame, make sure that a frame is allowed
> to egress on that port.

For egress checks it might be better to have per-vlan bitmaps of ports
instead. For unicast this doesn't really change anything, but for
broadcast and multicast all ports are checked so reading all bits from
one location is going to be cheaper.

[...]
> -static inline struct net_port_vlans *br_get_vlan_info(struct net_bridge *br)
> +static inline struct net_port_vlans *br_get_vlan_info(
> +                                               const struct net_bridge *br)
>  {
>         return rcu_dereference(br->vlan_info);
>  }
>
> -static inline struct net_port_vlans *nbp_get_vlan_info(struct net_bridge_port *p)
> +static inline struct net_port_vlans *nbp_get_vlan_info(
> +                                               const struct net_bridge_port *p)
>  {
>         return rcu_dereference(p->vlan_info);
>  }

Shouldn't this be in patch 1?

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vlad Yasevich - Jan. 31, 2013, 8:17 p.m.
On 01/31/2013 03:03 PM, Michał Mirosław wrote:
> 2013/1/31 Vlad Yasevich <vyasevic@redhat.com>:
>> When bridge forwards a frame, make sure that a frame is allowed
>> to egress on that port.
>
> For egress checks it might be better to have per-vlan bitmaps of ports
> instead. For unicast this doesn't really change anything, but for
> broadcast and multicast all ports are checked so reading all bits from
> one location is going to be cheaper.

But then one would have to locate that particular bitmap and now we are 
back to a list/hash of vlans each containing port bitmap.  Considering 
how strenuously Stephen wanted a bitmap of vlans, I'll leave it as is 
for now.

>
> [...]
>> -static inline struct net_port_vlans *br_get_vlan_info(struct net_bridge *br)
>> +static inline struct net_port_vlans *br_get_vlan_info(
>> +                                               const struct net_bridge *br)
>>   {
>>          return rcu_dereference(br->vlan_info);
>>   }
>>
>> -static inline struct net_port_vlans *nbp_get_vlan_info(struct net_bridge_port *p)
>> +static inline struct net_port_vlans *nbp_get_vlan_info(
>> +                                               const struct net_bridge_port *p)
>>   {
>>          return rcu_dereference(p->vlan_info);
>>   }
>
> Shouldn't this be in patch 1?

Sure.  'const' wasn't needed until this patch though, but I guess its 
good practice.

Thanks for taking a look
-vlad
>
> Best Regards,
> Michał Mirosław
>

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

Patch

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 02015a5..35b0671 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -31,6 +31,7 @@  static inline int should_deliver(const struct net_bridge_port *p,
 				 const struct sk_buff *skb)
 {
 	return (((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
+		br_allowed_egress(p->br, nbp_get_vlan_info(p), skb) &&
 		p->state == BR_STATE_FORWARDING);
 }
 
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 4ef3f6b..787d7da 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -35,6 +35,16 @@  static int br_pass_frame_up(struct sk_buff *skb)
 	brstats->rx_bytes += skb->len;
 	u64_stats_update_end(&brstats->syncp);
 
+	/* Bridge is just like any other port.  Make sure the
+	 * packet is allowed except in promisc modue when someone
+	 * may be running packet capture.
+	 */
+	if (!(brdev->flags & IFF_PROMISC) &&
+	    !br_allowed_egress(br, br_get_vlan_info(br), skb)) {
+		kfree_skb(skb);
+		return NET_RX_DROP;
+	}
+
 	indev = skb->dev;
 	skb->dev = brdev;
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index be9ba73..f9ee32f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -553,6 +553,9 @@  static inline void br_mdb_uninit(void)
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 extern bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 			       struct sk_buff *skb);
+extern bool br_allowed_egress(struct net_bridge *br,
+			      const struct net_port_vlans *v,
+			      const struct sk_buff *skb);
 extern int br_vlan_add(struct net_bridge *br, u16 vid);
 extern int br_vlan_delete(struct net_bridge *br, u16 vid);
 extern void br_vlan_flush(struct net_bridge *br);
@@ -561,12 +564,14 @@  extern int nbp_vlan_add(struct net_bridge_port *port, u16 vid);
 extern int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
 extern void nbp_vlan_flush(struct net_bridge_port *port);
 
-static inline struct net_port_vlans *br_get_vlan_info(struct net_bridge *br)
+static inline struct net_port_vlans *br_get_vlan_info(
+						const struct net_bridge *br)
 {
 	return rcu_dereference(br->vlan_info);
 }
 
-static inline struct net_port_vlans *nbp_get_vlan_info(struct net_bridge_port *p)
+static inline struct net_port_vlans *nbp_get_vlan_info(
+						const struct net_bridge_port *p)
 {
 	return rcu_dereference(p->vlan_info);
 }
@@ -590,6 +595,13 @@  static inline bool br_allowed_ingress(struct net_bridge *br,
 	return true;
 }
 
+static inline bool br_allowed_egress(struct net_bridge *br,
+				     const struct net_port_vlans *v,
+				     const struct sk_buff *skb)
+{
+	return true;
+}
+
 static inline int br_vlan_add(struct net_bridge *br, u16 vid)
 {
 	return -EOPNOTSUPP;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 815678b..912bc75 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -124,6 +124,26 @@  bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 	return false;
 }
 
+/* Called under RCU. */
+bool br_allowed_egress(struct net_bridge *br,
+		       const struct net_port_vlans *v,
+		       const struct sk_buff *skb)
+{
+	u16 vid;
+
+	if (!br->vlan_enabled)
+		return true;
+
+	if (!v)
+		return false;
+
+	vid = br_vlan_get_tag(skb);
+	if (test_bit(vid, v->vlan_bitmap))
+		return true;
+
+	return false;
+}
+
 /* Must be protected by RTNL */
 int br_vlan_add(struct net_bridge *br, u16 vid)
 {