[PATCH 2.6.36/stable] vlan: Fix crash when hwaccel rx pkt for non-existant vlan.

[PATCH 2.6.36/stable] vlan: Fix crash when hwaccel rx pkt for non-existant vlan.

[Ben Greear]

The vlan_hwaccel_do_receive code expected skb->dev to always
be a vlan device, but if the NIC was promisc, and the VLAN
for a particular VID was not configured, then this method
could receive a packet where skb->dev was NOT a vlan
device.  This caused access of bad memory and a crash.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

This passes some quick testing, but it needs review, as
I have not messed with the VLAN code in a while and might
be missing something.

:100644 100644 0eb96f7... 2883d0e... M	net/8021q/vlan_core.c
:100644 100644 660dd41... 5dc45b9... M	net/core/dev.c
 net/8021q/vlan_core.c |   12 +++++++++---
 net/core/dev.c        |    5 +++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 0eb96f7..2883d0e 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -43,10 +43,16 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
 	struct net_device *dev = skb->dev;
 	struct vlan_rx_stats     *rx_stats;
 
-	skb->dev = vlan_dev_info(dev)->real_dev;
-	netif_nit_deliver(skb);
+	if (is_vlan_dev(dev)) {
+		skb->dev = vlan_dev_info(dev)->real_dev;
+		netif_nit_deliver(skb);
+		skb->dev = dev;
+	} else {
+		netif_nit_deliver(skb);
+		skb->pkt_type = PACKET_OTHERHOST;
+		return 0;
+	}
 
-	skb->dev = dev;
 	skb->priority = vlan_get_ingress_priority(dev, skb->vlan_tci);
 	skb->vlan_tci = 0;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 660dd41..5dc45b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2828,8 +2828,9 @@ static int __netif_receive_skb(struct sk_buff *skb)
 	if (!netdev_tstamp_prequeue)
 		net_timestamp_check(skb);
 
-	if (vlan_tx_tag_present(skb) && vlan_hwaccel_do_receive(skb))
-		return NET_RX_SUCCESS;
+	if (vlan_tx_tag_present(skb))
+		/* This method cannot fail at this time. */
+		vlan_hwaccel_do_receive(skb);
 
 	/* if we've gotten here through NAPI, check netpoll */
 	if (netpoll_receive_skb(skb))
-- 
1.6.2.5

Re: [PATCH 2.6.36/stable] vlan: Fix crash when hwaccel rx pkt for non-existant vlan.

[Jesse Gross]

On Mon, Oct 25, 2010 at 3:52 PM, Ben Greear <greearb@candelatech.com> wrote:
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 0eb96f7..2883d0e 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -43,10 +43,16 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
>        struct net_device *dev = skb->dev;
>        struct vlan_rx_stats     *rx_stats;
>
> -       skb->dev = vlan_dev_info(dev)->real_dev;
> -       netif_nit_deliver(skb);
> +       if (is_vlan_dev(dev)) {
> +               skb->dev = vlan_dev_info(dev)->real_dev;
> +               netif_nit_deliver(skb);
> +               skb->dev = dev;
> +       } else {
> +               netif_nit_deliver(skb);
> +               skb->pkt_type = PACKET_OTHERHOST;
> +               return 0;
> +       }

Neither setting skb->pkt_type nor calling netif_nit_deliver() should
be necessary here.  The former is done on receive if there is no
matching device and the latter happens later on in
__netif_receive_skb().  So you should just be able to do:

if (!is_vlan_dev(dev))
    return 0;

One potentially bad outcome of this approach is that other components
like bridging can start to pick up packets with vlan tags in
skb->vlan_tci.  However, in 2.6.36 it isn't able to do filtering on
them and the NIC on the output side may not be able to handle the tag
in the skb.  It's probably not common to have both bridging and a vlan
group on the same device but you'll get into some weird situations if
you do.

Once you start solving these issues you end up with more invasive
changes that look like the implementation in 2.6.37.  Since we don't
want to pull all of that in, it might just be best to deliver the
packet to network taps and then drop it.  That was the original reason
for allowing vlan packets with no device this far into the stack.