Re: [PATCH net-next V4 03/13] bridge: Validate that vlan is permitted on ingress
From: Shmulik Ladkani <hidden>
Date: 2012-12-20 14:07:21
Hi Vlad, On Wed, 19 Dec 2012 12:48:14 -0500 Vlad Yasevich [off-list ref] wrote:
+static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
+{
+ struct net_port_vlan *pve;
+ u16 vid;
+
+ /* If there are no vlan in the permitted list, all packets are
+ * permitted.
+ */
+ if (list_empty(&p->vlan_list))
+ return true;I assumed the default policy would be Drop in such case, otherwise leaking between vlan domains is possible. Or maybe, ingress policy when port isn't a member of ingress VID should be configurable (drop/allow).
+ vid = br_get_vlan(skb); + pve = nbp_vlan_find(p, vid);
Why search by iterating through NBP's vlan_list? You know the VID (hence may fetch the net_bridge_vlan from the hash), so why don't you directly consult the net_bridge_vlan's port_bitmap?
quoted hunk ↗ jump to hunk
@@ -54,6 +74,9 @@ int br_handle_frame_finish(struct sk_buff *skb) if (!p || p->state == BR_STATE_DISABLED) goto drop; + if (!br_allowed_ingress(p, skb)) + goto drop; +
This condition should be also encorporated upon "ingress" at the "bridge master port" (that is, early at br_dev_xmit). Think of the "bridge master port" as yet another port: upon "ingress" (meaning, tx packets from the ip stack), we should also enforce any ingress permission rules. Regards, Shmulik