[patch 5/9] net pppoe: Check packet length on all receive paths

From: Greg KH
Date: Fri Jul 25 2008 - 19:12:30 EST


2.6.25-stable review patch. If anyone has any objections, please let us
know.

------------------
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

[ Upstream commit 392fdb0e35055b96faa9c1cd6ab537805337cdce ]

The length field in the PPPOE header wasn't checked completely.
This patch causes all packets shorter than the declared length
to be dropped.

It also changes the memcpy_toiovec call to skb_copy_datagram_iovec
so that paged packets (rare for PPPOE) are handled properly.

Thanks to Ilja of the Netric Security Team for discovering and
reporting this bug, and Chris Wright for the total_len check.

[ Incorporate warning fix from Stephen Hemminger. -DaveM ]

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>

---
drivers/net/pppoe.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)

--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -341,12 +341,6 @@ static int pppoe_rcv_core(struct sock *s
struct pppox_sock *relay_po;

if (sk->sk_state & PPPOX_BOUND) {
- struct pppoe_hdr *ph = pppoe_hdr(skb);
- int len = ntohs(ph->length);
- skb_pull_rcsum(skb, sizeof(struct pppoe_hdr));
- if (pskb_trim_rcsum(skb, len))
- goto abort_kfree;
-
ppp_input(&po->chan, skb);
} else if (sk->sk_state & PPPOX_RELAY) {
relay_po = get_item_by_addr(&po->pppoe_relay);
@@ -357,7 +351,6 @@ static int pppoe_rcv_core(struct sock *s
if ((sk_pppox(relay_po)->sk_state & PPPOX_CONNECTED) == 0)
goto abort_put;

- skb_pull(skb, sizeof(struct pppoe_hdr));
if (!__pppoe_xmit(sk_pppox(relay_po), skb))
goto abort_put;
} else {
@@ -388,6 +381,7 @@ static int pppoe_rcv(struct sk_buff *skb
{
struct pppoe_hdr *ph;
struct pppox_sock *po;
+ int len;

if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
goto out;
@@ -399,10 +393,21 @@ static int pppoe_rcv(struct sk_buff *skb
goto drop;

ph = pppoe_hdr(skb);
+ len = ntohs(ph->length);
+
+ skb_pull_rcsum(skb, sizeof(*ph));
+ if (skb->len < len)
+ goto drop;

po = get_item(ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
- if (po != NULL)
- return sk_receive_skb(sk_pppox(po), skb, 0);
+ if (!po)
+ goto drop;
+
+ if (pskb_trim_rcsum(skb, len))
+ goto drop;
+
+ return sk_receive_skb(sk_pppox(po), skb, 0);
+
drop:
kfree_skb(skb);
out:
@@ -937,12 +942,10 @@ static int pppoe_recvmsg(struct kiocb *i
m->msg_namelen = 0;

if (skb) {
- struct pppoe_hdr *ph = pppoe_hdr(skb);
- const int len = ntohs(ph->length);
-
- error = memcpy_toiovec(m->msg_iov, (unsigned char *) &ph->tag[0], len);
+ total_len = min_t(size_t, total_len, skb->len);
+ error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);
if (error == 0)
- error = len;
+ error = total_len;
}

kfree_skb(skb);

--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/