BUG: while bridging Ethernet and wireless device:

On Thu, 2010-12-16 at 14:11 +0200, Tomas Winkler wrote:
> Will be happy if someone can give me some more insight. (kernel 2.6.37-rc5)

Tomas looked into it a bit more and told me that it happens on IPv6
packets. To recap, he gets

kernel BUG at include/linux/skbuff.h:1178!
with
EIP: [<f83edd65>] br_multicast_rcv+0xc95/0xe1c [bridge]

Also remember that the packets are almost fully nonlinear, when they get
here they likely have almost no data in the skb header.

I then looked at br_multicast_ipv6_rcv(), and it looks fishy:

Up to:
	skb2 = skb_clone(skb, GFP_ATOMIC);

everything's fine, since ipv6_skip_exthdr() will use
skb_header_pointer(). At this point, offset is the result of
ipv6_skip_exthdr(). Remember that skb_clone() is not skb_copy().

Then, however, we do
	__skb_pull(skb2, offset);

At this point, however, I don't see anything that guarantees that all
"offset" bytes are part of the headroom -- and indeed I think this is
where it crashes.

If it didn't crash, because this many bytes were part of the header,
continuing further into the function, however, we could still crash:

        if (!pskb_may_pull(skb2, sizeof(*icmp6h)))
                goto out;

now makes sure that we can read the ICMPv6 header. Later, however, we do

        case ICMPV6_MGM_REPORT:
            {
                struct mld_msg *mld = (struct mld_msg *)icmp6h;
                BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
                err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);

which seems just as unsafe since "mld_mca" need not be part of the
header of the SKB. Similarly in another branch of this.

Additionally, I'm not convinced that there even is guaranteed to be
enough space in the SKB at all for the entire "struct mld_msg".

And finally, the error path in this function is confusing. Below patch
should be fine since unlike IPv4 (where this was copied maybe?) this
code unconditionally clones the SKB.

johannes

---
 net/bridge/br_multicast.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)




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

2010/12/29 Johannes Berg <johannes@sipsolutions.net>:
> On Thu, 2010-12-16 at 14:11 +0200, Tomas Winkler wrote:
>> Will be happy if someone can give me some more insight. (kernel 2.6.37-rc5)
>
> Tomas looked into it a bit more and told me that it happens on IPv6
> packets. To recap, he gets
>
> kernel BUG at include/linux/skbuff.h:1178!
> with
> EIP: [<f83edd65>] br_multicast_rcv+0xc95/0xe1c [bridge]
>
> Also remember that the packets are almost fully nonlinear, when they get
> here they likely have almost no data in the skb header.
>
> I then looked at br_multicast_ipv6_rcv(), and it looks fishy:
>
> Up to:
>        skb2 = skb_clone(skb, GFP_ATOMIC);
>
> everything's fine, since ipv6_skip_exthdr() will use
> skb_header_pointer(). At this point, offset is the result of
> ipv6_skip_exthdr(). Remember that skb_clone() is not skb_copy().

So far I can confirm that switching to sbk_copy fixes the crash.

Thanks
Tomas
--
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