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

[PATCH 2.6.36/stable v2] 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>
---
v1 -> v2:  Simplify patch..no need for setting pkt-type, etc.

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

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 0eb96f7..0687b6c 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -43,6 +43,9 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
 	struct net_device *dev = skb->dev;
 	struct vlan_rx_stats     *rx_stats;
 
+	if (!is_vlan_dev(dev))
+		return 0;
+
 	skb->dev = vlan_dev_info(dev)->real_dev;
 	netif_nit_deliver(skb);
 
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 v2] vlan: Fix crash when hwaccel rx pkt for non-existant vlan.

[Jesse Gross]

On Tue, Oct 26, 2010 at 10:06 AM, Ben Greear <greearb@candelatech.com> wrote:
> 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>
> ---
> v1 -> v2:  Simplify patch..no need for setting pkt-type, etc.
>
> :100644 100644 0eb96f7... 0687b6c... M  net/8021q/vlan_core.c
> :100644 100644 660dd41... 5dc45b9... M  net/core/dev.c
>  net/8021q/vlan_core.c |    3 +++
>  net/core/dev.c        |    5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 0eb96f7..0687b6c 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -43,6 +43,9 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
>        struct net_device *dev = skb->dev;
>        struct vlan_rx_stats     *rx_stats;
>
> +       if (!is_vlan_dev(dev))
> +               return 0;
> +
>        skb->dev = vlan_dev_info(dev)->real_dev;
>        netif_nit_deliver(skb);
>

What if we dropped any packet with a tag in skb->vlan_tci before it
gets to the bridge hooks?  That would accomplish the original goal of
getting packets to tcpdump while preventing them from making it to
places where they aren't expected,  It will provide the same behavior
as earlier kernels.

> 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);

This is correct but it's not a bugfix, so I'm not sure that it should
go to -stable.  It's already been fixed for 2.6.37.

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

[Ben Greear]

On 10/27/2010 05:11 PM, Jesse Gross wrote:
> On Tue, Oct 26, 2010 at 10:06 AM, Ben Greear<greearb@candelatech.com>  wrote:
>> 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>
>> ---
>> v1 ->  v2:  Simplify patch..no need for setting pkt-type, etc.
>>
>> :100644 100644 0eb96f7... 0687b6c... M  net/8021q/vlan_core.c
>> :100644 100644 660dd41... 5dc45b9... M  net/core/dev.c
>>   net/8021q/vlan_core.c |    3 +++
>>   net/core/dev.c        |    5 +++--
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> index 0eb96f7..0687b6c 100644
>> --- a/net/8021q/vlan_core.c
>> +++ b/net/8021q/vlan_core.c
>> @@ -43,6 +43,9 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
>>         struct net_device *dev = skb->dev;
>>         struct vlan_rx_stats     *rx_stats;
>>
>> +       if (!is_vlan_dev(dev))
>> +               return 0;
>> +
>>         skb->dev = vlan_dev_info(dev)->real_dev;
>>         netif_nit_deliver(skb);
>>
>
> What if we dropped any packet with a tag in skb->vlan_tci before it
> gets to the bridge hooks?  That would accomplish the original goal of
> getting packets to tcpdump while preventing them from making it to
> places where they aren't expected,  It will provide the same behavior
> as earlier kernels.

The VLAN code has changed a lot since I messed with it last, so
there very well may be better ways to fix this than what I came up
with.  Please propose a patch if you have a suggestion.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com