Message ID | 1282809788.2476.59.camel@edumazet-laptop |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2010-08-26 at 09:03 +0100, Eric Dumazet wrote: > Here is the patch, could you test it please ? > > Thanks ! > > [PATCH] l2tp: test for malicious frames in l2tp_eth_dev_recv() > > close https://bugzilla.kernel.org/show_bug.cgi?id=16529 > > Before calling dev_forward_skb(), we should make sure skb contains at > least an ethernet header, even if length included in upper layer said > so. Does this imply that there is some problem with xen-netfront setting skb->len or skb->data_len or something incorrectly? It's not clear where data_len has come from in this context. Ian. > > Reported-by: Thomas Heil <heil@terminal-consulting.de> > Reported-by: Ian Campbell <Ian.Campbell@eu.citrix.com> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > net/l2tp/l2tp_core.c | 2 +- > net/l2tp/l2tp_eth.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c > index 58c6c4c..0687c5c 100644 > --- a/net/l2tp/l2tp_eth.c > +++ b/net/l2tp/l2tp_eth.c > @@ -132,7 +132,7 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb, > printk("\n"); > } > > - if (data_len < ETH_HLEN) > + if (skb->len < ETH_HLEN) > goto error; > > secpath_reset(skb); > > -- 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
Le jeudi 26 août 2010 à 09:14 +0100, Ian Campbell a écrit : > On Thu, 2010-08-26 at 09:03 +0100, Eric Dumazet wrote: > > Here is the patch, could you test it please ? > > > > Thanks ! > > > > [PATCH] l2tp: test for malicious frames in l2tp_eth_dev_recv() > > > > close https://bugzilla.kernel.org/show_bug.cgi?id=16529 > > > > Before calling dev_forward_skb(), we should make sure skb contains at > > least an ethernet header, even if length included in upper layer said > > so. > > Does this imply that there is some problem with xen-netfront setting > skb->len or skb->data_len or something incorrectly? It's not clear where > data_len has come from in this context. data_len is a 16bit field provided in a prior encapsulation header, provided by user (untrusted source) Some buggy or malicious software sent an invalid frame, < encapsulation [len=1000] > < 'runt' eth frame (len<14) > Another fix would be to change l2tp_recv_dequeue_skb(), and check L2TP_SKB_CB(skb)->length against skb->len, before calling (*session->recv_skb)(session, skb, length); I prefer the one liner patch I sent you, as a minimum fix. -- 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
On Thu, 2010-08-26 at 09:34 +0100, Eric Dumazet wrote: > Le jeudi 26 août 2010 à 09:14 +0100, Ian Campbell a écrit : > > On Thu, 2010-08-26 at 09:03 +0100, Eric Dumazet wrote: > > > Here is the patch, could you test it please ? > > > > > > Thanks ! > > > > > > [PATCH] l2tp: test for malicious frames in l2tp_eth_dev_recv() > > > > > > close https://bugzilla.kernel.org/show_bug.cgi?id=16529 > > > > > > Before calling dev_forward_skb(), we should make sure skb contains at > > > least an ethernet header, even if length included in upper layer said > > > so. > > > > Does this imply that there is some problem with xen-netfront setting > > skb->len or skb->data_len or something incorrectly? It's not clear where > > data_len has come from in this context. > > data_len is a 16bit field provided in a prior encapsulation header, > provided by user (untrusted source) > > Some buggy or malicious software sent an invalid frame, > > > < encapsulation [len=1000] > < 'runt' eth frame (len<14) > > > Another fix would be to change l2tp_recv_dequeue_skb(), and check > > L2TP_SKB_CB(skb)->length against skb->len, before calling > > (*session->recv_skb)(session, skb, length); > > > I prefer the one liner patch I sent you, as a minimum fix. Thanks, I just wanted to be sure we weren't papering over a potential issue in xen-netfront. Ian. -- 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
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index 58c6c4c..0687c5c 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -132,7 +132,7 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb, printk("\n"); } - if (data_len < ETH_HLEN) + if (skb->len < ETH_HLEN) goto error; secpath_reset(skb);
Here is the patch, could you test it please ? Thanks ! [PATCH] l2tp: test for malicious frames in l2tp_eth_dev_recv() close https://bugzilla.kernel.org/show_bug.cgi?id=16529 Before calling dev_forward_skb(), we should make sure skb contains at least an ethernet header, even if length included in upper layer said so. Reported-by: Thomas Heil <heil@terminal-consulting.de> Reported-by: Ian Campbell <Ian.Campbell@eu.citrix.com> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/l2tp/l2tp_core.c | 2 +- net/l2tp/l2tp_eth.c | 2 +- 2 files changed, 2 insertions(+), 2 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