From: Herbert Xu Date: Tue, 18 Sep 2007 20:18:42 +0000 (-0700) Subject: [PPP] L2TP: Fix skb handling in pppol2tp_recv_core X-Git-Tag: v2.6.23-rc8~19^2~2 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a70e39b6633ad85936b029463134ee2599600f1;p=linux-2.6 [PPP] L2TP: Fix skb handling in pppol2tp_recv_core The function pppol2tp_recv_core doesn't handle non-linear packets properly. It also fails to check the remote offset field. This patch fixes these problems. It also removes an unnecessary check on the UDP header which has already been performed by the UDP layer. Signed-off-by: Herbert Xu Signed-off-by: David S. Miller --- diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c index ed8ead432d..440e190778 100644 --- a/drivers/net/pppol2tp.c +++ b/drivers/net/pppol2tp.c @@ -491,44 +491,46 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) u16 hdrflags; u16 tunnel_id, session_id; int length; - struct udphdr *uh; + int offset; tunnel = pppol2tp_sock_to_tunnel(sock); if (tunnel == NULL) goto error; + /* UDP always verifies the packet length. */ + __skb_pull(skb, sizeof(struct udphdr)); + /* Short packet? */ - if (skb->len < sizeof(struct udphdr)) { + if (!pskb_may_pull(skb, 12)) { PRINTK(tunnel->debug, PPPOL2TP_MSG_DATA, KERN_INFO, "%s: recv short packet (len=%d)\n", tunnel->name, skb->len); goto error; } /* Point to L2TP header */ - ptr = skb->data + sizeof(struct udphdr); + ptr = skb->data; /* Get L2TP header flags */ hdrflags = ntohs(*(__be16*)ptr); /* Trace packet contents, if enabled */ if (tunnel->debug & PPPOL2TP_MSG_DATA) { + length = min(16u, skb->len); + if (!pskb_may_pull(skb, length)) + goto error; + printk(KERN_DEBUG "%s: recv: ", tunnel->name); - for (length = 0; length < 16; length++) - printk(" %02X", ptr[length]); + offset = 0; + do { + printk(" %02X", ptr[offset]); + } while (++offset < length); + printk("\n"); } /* Get length of L2TP packet */ - uh = (struct udphdr *) skb_transport_header(skb); - length = ntohs(uh->len) - sizeof(struct udphdr); - - /* Too short? */ - if (length < 12) { - PRINTK(tunnel->debug, PPPOL2TP_MSG_DATA, KERN_INFO, - "%s: recv short L2TP packet (len=%d)\n", tunnel->name, length); - goto error; - } + length = skb->len; /* If type is control packet, it is handled by userspace. */ if (hdrflags & L2TP_HDRFLAG_T) { @@ -606,7 +608,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) "%s: recv data has no seq numbers when required. " "Discarding\n", session->name); session->stats.rx_seq_discards++; - session->stats.rx_errors++; goto discard; } @@ -625,7 +626,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) "%s: recv data has no seq numbers when required. " "Discarding\n", session->name); session->stats.rx_seq_discards++; - session->stats.rx_errors++; goto discard; } @@ -634,10 +634,14 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) } /* If offset bit set, skip it. */ - if (hdrflags & L2TP_HDRFLAG_O) - ptr += 2 + ntohs(*(__be16 *) ptr); + if (hdrflags & L2TP_HDRFLAG_O) { + offset = ntohs(*(__be16 *)ptr); + skb->transport_header += 2 + offset; + if (!pskb_may_pull(skb, skb_transport_offset(skb) + 2)) + goto discard; + } - skb_pull(skb, ptr - skb->data); + __skb_pull(skb, skb_transport_offset(skb)); /* Skip PPP header, if present. In testing, Microsoft L2TP clients * don't send the PPP header (PPP header compression enabled), but @@ -673,7 +677,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) */ if (PPPOL2TP_SKB_CB(skb)->ns != session->nr) { session->stats.rx_seq_discards++; - session->stats.rx_errors++; PRINTK(session->debug, PPPOL2TP_MSG_SEQ, KERN_DEBUG, "%s: oos pkt %hu len %d discarded, " "waiting for %hu, reorder_q_len=%d\n", @@ -698,6 +701,7 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb) return 0; discard: + session->stats.rx_errors++; kfree_skb(skb); sock_put(session->sock);