lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <731d94aa-b8a6-1ae4-853d-0a9a24dc698f@oracle.com> Date: Thu, 24 Jan 2019 15:51:17 +0800 From: Jacob Wen <jian.w.wen@...cle.com> To: Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org Cc: jchapman@...alix.com Subject: Re: [PATCH net-next v1] l2tp: fix reading optional fields On 1/23/19 10:51 AM, Eric Dumazet wrote: > > On 01/22/2019 06:30 PM, Jacob Wen wrote: >> Use pskb_may_pull() to make sure the optional fields are in skb linear >> parts. >> >> Signed-off-by: Jacob Wen <jian.w.wen@...cle.com> >> --- >> v1: fix warning: ISO C90 forbids mixed declarations and code >> --- >> net/l2tp/l2tp_core.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c >> index 26f1d435696a..c0dfa2bcd218 100644 >> --- a/net/l2tp/l2tp_core.c >> +++ b/net/l2tp/l2tp_core.c >> @@ -627,6 +627,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, >> >> /* Parse and check optional cookie */ >> if (session->peer_cookie_len > 0) { >> + if (!pskb_may_pull(skb, ptr - optr + session->peer_cookie_len)) >> + goto discard; > This is completely buggy. > > After pskb_may_pull(), the ptr and optr pointers might point to freed data. > > So the memcmp() might crash hard. Fixed in v2. Thanks. > >> if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) { >> l2tp_info(tunnel, L2TP_MSG_DATA, >> "%s: cookie mismatch (%u/%u). Discarding.\n", >> @@ -649,6 +651,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, >> L2TP_SKB_CB(skb)->has_seq = 0; >> if (tunnel->version == L2TP_HDR_VER_2) { >> if (hdrflags & L2TP_HDRFLAG_S) { >> + if (!pskb_may_pull(skb, ptr - optr + 4)) >> + goto discard; >> ns = ntohs(*(__be16 *) ptr); >> ptr += 2; >> nr = ntohs(*(__be16 *) ptr); >> @@ -663,8 +667,12 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, >> session->name, ns, nr, session->nr); >> } >> } else if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) { >> - u32 l2h = ntohl(*(__be32 *) ptr); >> + u32 l2h; >> + >> + if (!pskb_may_pull(skb, ptr - optr + 4)) >> + goto discard; >> >> + l2h = ntohl(*(__be32 *) ptr); >> if (l2h & 0x40000000) { >> ns = l2h & 0x00ffffff; >> >> @@ -729,6 +737,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, >> if (tunnel->version == L2TP_HDR_VER_2) { >> /* If offset bit set, skip it. */ >> if (hdrflags & L2TP_HDRFLAG_O) { >> + if (!pskb_may_pull(skb, ptr - optr + 2)) >> + goto discard; >> offset = ntohs(*(__be16 *)ptr); >> ptr += 2 + offset; >> } >> -- Jacob
Powered by blists - more mailing lists