diff mbox series

[net] ip: on queued skb use skb_header_pointer instead of pskb_may_pull

Message ID 20190107214733.234219-1-willemdebruijn.kernel@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net] ip: on queued skb use skb_header_pointer instead of pskb_may_pull | expand

Commit Message

Willem de Bruijn Jan. 7, 2019, 9:47 p.m. UTC 
From: Willem de Bruijn <willemb@google.com>

Commit 2efd4fca703a ("ip: in cmsg IP(V6)_ORIGDSTADDR call
pskb_may_pull") avoided a read beyond the end of the skb linear
segment by calling pskb_may_pull.

That function can trigger a BUG_ON in pskb_expand_head if the skb is
shared, which it is when when peeking. It can also return ENOMEM.

Avoid both by switching to safer skb_header_pointer.

Fixes: 2efd4fca703a ("ip: in cmsg IP(V6)_ORIGDSTADDR call pskb_may_pull")
Reported-by: syzbot <syzkaller@googlegroups.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/ipv4/ip_sockglue.c | 12 +++++-------
 net/ipv6/datagram.c    | 10 ++++------
 2 files changed, 9 insertions(+), 13 deletions(-)

Comments

David Miller Jan. 10, 2019, 2:28 p.m. UTC | #1 
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Mon,  7 Jan 2019 16:47:33 -0500

> From: Willem de Bruijn <willemb@google.com>
> 
> Commit 2efd4fca703a ("ip: in cmsg IP(V6)_ORIGDSTADDR call
> pskb_may_pull") avoided a read beyond the end of the skb linear
> segment by calling pskb_may_pull.
> 
> That function can trigger a BUG_ON in pskb_expand_head if the skb is
> shared, which it is when when peeking. It can also return ENOMEM.
> 
> Avoid both by switching to safer skb_header_pointer.
> 
> Fixes: 2efd4fca703a ("ip: in cmsg IP(V6)_ORIGDSTADDR call pskb_may_pull")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied and queued up for -stable.
diff mbox series

Patch

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index fffcc130900e..82f341e84fae 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -148,19 +148,17 @@  static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
 
 static void ip_cmsg_recv_dstaddr(struct msghdr *msg, struct sk_buff *skb)
 {
+	__be16 _ports[2], *ports;
 	struct sockaddr_in sin;
-	__be16 *ports;
-	int end;
-
-	end = skb_transport_offset(skb) + 4;
-	if (end > 0 && !pskb_may_pull(skb, end))
-		return;
 
 	/* All current transport protocols have the port numbers in the
 	 * first four bytes of the transport header and this function is
 	 * written with this assumption in mind.
 	 */
-	ports = (__be16 *)skb_transport_header(skb);
+	ports = skb_header_pointer(skb, skb_transport_offset(skb),
+				   sizeof(_ports), &_ports);
+	if (!ports)
+		return;
 
 	sin.sin_family = AF_INET;
 	sin.sin_addr.s_addr = ip_hdr(skb)->daddr;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index bde08aa549f3..c2262a7e2088 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -700,17 +700,15 @@  void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	}
 	if (np->rxopt.bits.rxorigdstaddr) {
 		struct sockaddr_in6 sin6;
-		__be16 *ports;
-		int end;
+		__be16 _ports[2], *ports;
 
-		end = skb_transport_offset(skb) + 4;
-		if (end <= 0 || pskb_may_pull(skb, end)) {
+		ports = skb_header_pointer(skb, skb_transport_offset(skb),
+					   sizeof(_ports), &_ports);
+		if (ports) {
 			/* All current transport protocols have the port numbers in the
 			 * first four bytes of the transport header and this function is
 			 * written with this assumption in mind.
 			 */
-			ports = (__be16 *)skb_transport_header(skb);
-
 			sin6.sin6_family = AF_INET6;
 			sin6.sin6_addr = ipv6_hdr(skb)->daddr;
 			sin6.sin6_port = ports[1];