net pppoe: Check packet length on all receive paths
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. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
ea23ec2672
commit
392fdb0e35
|
@ -341,12 +341,6 @@ static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
|
||||||
struct pppox_sock *relay_po;
|
struct pppox_sock *relay_po;
|
||||||
|
|
||||||
if (sk->sk_state & PPPOX_BOUND) {
|
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);
|
ppp_input(&po->chan, skb);
|
||||||
} else if (sk->sk_state & PPPOX_RELAY) {
|
} else if (sk->sk_state & PPPOX_RELAY) {
|
||||||
relay_po = get_item_by_addr(&po->pppoe_relay);
|
relay_po = get_item_by_addr(&po->pppoe_relay);
|
||||||
|
@ -357,7 +351,6 @@ static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
|
||||||
if ((sk_pppox(relay_po)->sk_state & PPPOX_CONNECTED) == 0)
|
if ((sk_pppox(relay_po)->sk_state & PPPOX_CONNECTED) == 0)
|
||||||
goto abort_put;
|
goto abort_put;
|
||||||
|
|
||||||
skb_pull(skb, sizeof(struct pppoe_hdr));
|
|
||||||
if (!__pppoe_xmit(sk_pppox(relay_po), skb))
|
if (!__pppoe_xmit(sk_pppox(relay_po), skb))
|
||||||
goto abort_put;
|
goto abort_put;
|
||||||
} else {
|
} else {
|
||||||
|
@ -388,6 +381,7 @@ static int pppoe_rcv(struct sk_buff *skb,
|
||||||
{
|
{
|
||||||
struct pppoe_hdr *ph;
|
struct pppoe_hdr *ph;
|
||||||
struct pppox_sock *po;
|
struct pppox_sock *po;
|
||||||
|
int len;
|
||||||
|
|
||||||
if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
|
if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -399,10 +393,21 @@ static int pppoe_rcv(struct sk_buff *skb,
|
||||||
goto drop;
|
goto drop;
|
||||||
|
|
||||||
ph = pppoe_hdr(skb);
|
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);
|
po = get_item(ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
|
||||||
if (po != NULL)
|
if (!po)
|
||||||
|
goto drop;
|
||||||
|
|
||||||
|
if (pskb_trim_rcsum(skb, len))
|
||||||
|
goto drop;
|
||||||
|
|
||||||
return sk_receive_skb(sk_pppox(po), skb, 0);
|
return sk_receive_skb(sk_pppox(po), skb, 0);
|
||||||
|
|
||||||
drop:
|
drop:
|
||||||
kfree_skb(skb);
|
kfree_skb(skb);
|
||||||
out:
|
out:
|
||||||
|
@ -937,12 +942,10 @@ static int pppoe_recvmsg(struct kiocb *iocb, struct socket *sock,
|
||||||
m->msg_namelen = 0;
|
m->msg_namelen = 0;
|
||||||
|
|
||||||
if (skb) {
|
if (skb) {
|
||||||
struct pppoe_hdr *ph = pppoe_hdr(skb);
|
total_len = min(total_len, skb->len);
|
||||||
const int len = ntohs(ph->length);
|
error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);
|
||||||
|
|
||||||
error = memcpy_toiovec(m->msg_iov, (unsigned char *) &ph->tag[0], len);
|
|
||||||
if (error == 0)
|
if (error == 0)
|
||||||
error = len;
|
error = total_len;
|
||||||
}
|
}
|
||||||
|
|
||||||
kfree_skb(skb);
|
kfree_skb(skb);
|
||||||
|
|
Loading…
Reference in a new issue