ip: on queued skb use skb_header_pointer instead of pskb_may_pull

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>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index fffcc13..82f341e 100644 (file)
--- 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 bde08aa..c2262a7 100644 (file)
--- 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];