If skb consists of several fragments this check fails net/llc/llc_input.c: ============================================= llc_fixup_skb() ... pdu = (struct llc_pdu_un *)skb->data; ... __be16 pdulen = eth_hdr(skb)->h_proto; s32 data_size = ntohs(pdulen) - llc_len; if (data_size < 0 || ((skb_tail_pointer(skb) - (u8 *)pdu) - llc_len) < data_size) return 0; ============================================= and packet is silently dropped. This breaks GVRP protocol if received packet is large ( > 512 bytes on my system) and contains a Leave All message. Since Leave All is missed, corresponding JoinIns are not sent and switch unregisters VLANs from port. Attached patch seems to resolve this issue, but I don't know if it's a correct solution.
Created attachment 53812 [details] patch
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface). On Fri, 8 Apr 2011 09:52:34 GMT bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=32872 > > Summary: LLC PDU is dropped if skb is not linear > Product: Networking > Version: 2.5 > Kernel Version: 2.6.32 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: Other > AssignedTo: acme@ghostprotocols.net > ReportedBy: vitalyb@telenet.dn.ua > Regression: No > > > If skb consists of several fragments this check fails > net/llc/llc_input.c: > ============================================= > llc_fixup_skb() > ... > pdu = (struct llc_pdu_un *)skb->data; > ... > __be16 pdulen = eth_hdr(skb)->h_proto; > s32 data_size = ntohs(pdulen) - llc_len; > > if (data_size < 0 || > ((skb_tail_pointer(skb) - > (u8 *)pdu) - llc_len) < data_size) > return 0; > ============================================= > > and packet is silently dropped. > > This breaks GVRP protocol if received packet is large ( > 512 bytes on my > system) and contains a Leave All message. Since Leave All is missed, > corresponding JoinIns are not sent and switch unregisters VLANs from port. > > Attached patch seems to resolve this issue, but I don't know if it's a > correct > solution. --- linux-2.6.32.36/net/llc/llc_input.c.orig 2009-12-03 05:51:21.000000000 +0200 +++ linux-2.6.32.36/net/llc/llc_input.c 2011-04-08 08:57:29.000000000 +0300 @@ -105,6 +105,11 @@ if (unlikely(!pskb_may_pull(skb, sizeof(*pdu)))) return 0; + if (skb->data_len != 0){ + if (unlikely(skb_linearize(skb))) + return 0; + } + pdu = (struct llc_pdu_un *)skb->data; if ((pdu->ctrl_1 & LLC_PDU_TYPE_MASK) == LLC_PDU_TYPE_U) llc_len = 1; 2.6.32 is a pretty old kernel - we'll need to verify if current kernels have the same problem. Please don't send patches via bugzilla - it causes lots of problems with our usual patch management and review processes. It's preferred that patches be sent via email as per Documentation/SubmittingPatches, and that they include a Signed-off-by:, as described in that file. Thanks.
From: Andrew Morton <akpm@linux-foundation.org> Date: Mon, 11 Apr 2011 16:48:12 -0700 > --- linux-2.6.32.36/net/llc/llc_input.c.orig 2009-12-03 05:51:21.000000000 > +0200 > +++ linux-2.6.32.36/net/llc/llc_input.c 2011-04-08 08:57:29.000000000 > +0300 > @@ -105,6 +105,11 @@ > if (unlikely(!pskb_may_pull(skb, sizeof(*pdu)))) > return 0; > > + if (skb->data_len != 0){ > + if (unlikely(skb_linearize(skb))) > + return 0; > + } > + > pdu = (struct llc_pdu_un *)skb->data; > if ((pdu->ctrl_1 & LLC_PDU_TYPE_MASK) == LLC_PDU_TYPE_U) > llc_len = 1; > > > 2.6.32 is a pretty old kernel - we'll need to verify if current kernels > have the same problem. > > Please don't send patches via bugzilla - it causes lots of problems > with our usual patch management and review processes. It's preferred > that patches be sent via email as per Documentation/SubmittingPatches, > and that they include a Signed-off-by:, as described in that file. The skb_tail_pointer() check in llc_fixup_skb() is beyond wonky and honestly the source of the problems here. I'd suggest instead: diff --git a/net/llc/llc_input.c b/net/llc/llc_input.c index 058f1e9..9032421 100644 --- a/net/llc/llc_input.c +++ b/net/llc/llc_input.c @@ -121,8 +121,7 @@ static inline int llc_fixup_skb(struct sk_buff *skb) s32 data_size = ntohs(pdulen) - llc_len; if (data_size < 0 || - ((skb_tail_pointer(skb) - - (u8 *)pdu) - llc_len) < data_size) + !pskb_may_pull(skb, data_size)) return 0; if (unlikely(pskb_trim_rcsum(skb, data_size))) return 0;
A patch referencing this bug report has been merged in v2.6.39-rc5: commit aa8673599f1d269b4e4d9b0c0f61fca57bc02699 Author: David S. Miller <davem@davemloft.net> Date: Mon Apr 11 18:59:05 2011 -0700 llc: Fix length check in llc_fixup_skb().