* [PATCH net-next v1] l2tp: fix reading optional fields
@ 2019-01-23 2:30 Jacob Wen
2019-01-23 2:51 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Jacob Wen @ 2019-01-23 2:30 UTC (permalink / raw)
To: netdev; +Cc: jchapman
Use pskb_may_pull() to make sure the optional fields are in skb linear
parts.
Signed-off-by: Jacob Wen <jian.w.wen@oracle.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;
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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next v1] l2tp: fix reading optional fields
2019-01-23 2:30 [PATCH net-next v1] l2tp: fix reading optional fields Jacob Wen
@ 2019-01-23 2:51 ` Eric Dumazet
2019-01-24 7:51 ` Jacob Wen
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2019-01-23 2:51 UTC (permalink / raw)
To: Jacob Wen, netdev; +Cc: jchapman
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@oracle.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.
> 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;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next v1] l2tp: fix reading optional fields
2019-01-23 2:51 ` Eric Dumazet
@ 2019-01-24 7:51 ` Jacob Wen
0 siblings, 0 replies; 3+ messages in thread
From: Jacob Wen @ 2019-01-24 7:51 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: jchapman
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@oracle.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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-24 7:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-23 2:30 [PATCH net-next v1] l2tp: fix reading optional fields Jacob Wen
2019-01-23 2:51 ` Eric Dumazet
2019-01-24 7:51 ` Jacob Wen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox