All of lore.kernel.org
 help / color / mirror / Atom feed
* pull request (net): ipsec 2018-07-27
@ 2018-07-27  6:51 Steffen Klassert
  2018-07-27  6:51 ` [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit Steffen Klassert
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix PMTU handling of vti6. We update the PMTU on
   the xfrm dst_entry which is not cached anymore
   after the flowchache removal. So update the
   PMTU of the original dst_entry instead.
   From Eyal Birger.

2) Fix a leak of kernel memory to userspace.
   From Eric Dumazet.

3) Fix a possible dst_entry memleak in xfrm_lookup_route.
   From Tommi Rantala.

4) Fix a skb leak in case we can't call nlmsg_multicast
   from xfrm_nlmsg_multicast. From Florian Westphal.

5) Fix a leak of a temporary buffer in the error path of
   esp6_input. From Zhen Lei.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 1c8c5a9d38f607c0b6fd12c91cbe1a4418762a21:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next (2018-06-06 18:39:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 7284fdf39a912322ce97de2d30def3c6068a418c:

  esp6: fix memleak on error path in esp6_input (2018-06-27 17:32:11 +0200)

----------------------------------------------------------------
Eric Dumazet (1):
      xfrm_user: prevent leaking 2 bytes of kernel memory

Eyal Birger (1):
      vti6: fix PMTU caching and reporting on xmit

Florian Westphal (1):
      xfrm: free skb if nlsk pointer is NULL

Tommi Rantala (1):
      xfrm: fix missing dst_release() after policy blocking lbcast and multicast

Zhen Lei (1):
      esp6: fix memleak on error path in esp6_input

 net/ipv6/esp6.c        |  4 +++-
 net/ipv6/ip6_vti.c     | 11 ++++++-----
 net/xfrm/xfrm_policy.c |  3 +++
 net/xfrm/xfrm_user.c   | 18 +++++++++++-------
 4 files changed, 23 insertions(+), 13 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
@ 2018-07-27  6:51 ` Steffen Klassert
  2018-07-27  6:51 ` [PATCH 2/5] xfrm_user: prevent leaking 2 bytes of kernel memory Steffen Klassert
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eyal Birger <eyal.birger@gmail.com>

When setting the skb->dst before doing the MTU check, the route PMTU
caching and reporting is done on the new dst which is about to be
released.

Instead, PMTU handling should be done using the original dst.

This is aligned with IPv4 VTI.

Fixes: ccd740cbc6 ("vti6: Add pmtu handling to vti6_xmit.")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/ip6_vti.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index b7f28deddaea..c72ae3a4fe09 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -480,10 +480,6 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
 		goto tx_err_dst_release;
 	}
 
-	skb_scrub_packet(skb, !net_eq(t->net, dev_net(dev)));
-	skb_dst_set(skb, dst);
-	skb->dev = skb_dst(skb)->dev;
-
 	mtu = dst_mtu(dst);
 	if (!skb->ignore_df && skb->len > mtu) {
 		skb_dst_update_pmtu(skb, mtu);
@@ -498,9 +494,14 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
 				  htonl(mtu));
 		}
 
-		return -EMSGSIZE;
+		err = -EMSGSIZE;
+		goto tx_err_dst_release;
 	}
 
+	skb_scrub_packet(skb, !net_eq(t->net, dev_net(dev)));
+	skb_dst_set(skb, dst);
+	skb->dev = skb_dst(skb)->dev;
+
 	err = dst_output(t->net, skb->sk, skb);
 	if (net_xmit_eval(err) == 0) {
 		struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] xfrm_user: prevent leaking 2 bytes of kernel memory
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
  2018-07-27  6:51 ` [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit Steffen Klassert
@ 2018-07-27  6:51 ` Steffen Klassert
  2018-07-27  6:51 ` [PATCH 3/5] xfrm: fix missing dst_release() after policy blocking lbcast and multicast Steffen Klassert
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eric Dumazet <edumazet@google.com>

struct xfrm_userpolicy_type has two holes, so we should not
use C99 style initializer.

KMSAN report:

BUG: KMSAN: kernel-infoleak in copyout lib/iov_iter.c:140 [inline]
BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
CPU: 1 PID: 4520 Comm: syz-executor841 Not tainted 4.17.0+ #5
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x185/0x1d0 lib/dump_stack.c:113
 kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:1117
 kmsan_internal_check_memory+0x138/0x1f0 mm/kmsan/kmsan.c:1211
 kmsan_copy_to_user+0x7a/0x160 mm/kmsan/kmsan.c:1253
 copyout lib/iov_iter.c:140 [inline]
 _copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
 copy_to_iter include/linux/uio.h:106 [inline]
 skb_copy_datagram_iter+0x422/0xfa0 net/core/datagram.c:431
 skb_copy_datagram_msg include/linux/skbuff.h:3268 [inline]
 netlink_recvmsg+0x6f1/0x1900 net/netlink/af_netlink.c:1959
 sock_recvmsg_nosec net/socket.c:802 [inline]
 sock_recvmsg+0x1d6/0x230 net/socket.c:809
 ___sys_recvmsg+0x3fe/0x810 net/socket.c:2279
 __sys_recvmmsg+0x58e/0xe30 net/socket.c:2391
 do_sys_recvmmsg+0x2a6/0x3e0 net/socket.c:2472
 __do_sys_recvmmsg net/socket.c:2485 [inline]
 __se_sys_recvmmsg net/socket.c:2481 [inline]
 __x64_sys_recvmmsg+0x15d/0x1c0 net/socket.c:2481
 do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x446ce9
RSP: 002b:00007fc307918db8 EFLAGS: 00000293 ORIG_RAX: 000000000000012b
RAX: ffffffffffffffda RBX: 00000000006dbc24 RCX: 0000000000446ce9
RDX: 000000000000000a RSI: 0000000020005040 RDI: 0000000000000003
RBP: 00000000006dbc20 R08: 0000000020004e40 R09: 0000000000000000
R10: 0000000040000000 R11: 0000000000000293 R12: 0000000000000000
R13: 00007ffc8d2df32f R14: 00007fc3079199c0 R15: 0000000000000001

Uninit was stored to memory at:
 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:279 [inline]
 kmsan_save_stack mm/kmsan/kmsan.c:294 [inline]
 kmsan_internal_chain_origin+0x12b/0x210 mm/kmsan/kmsan.c:685
 kmsan_memcpy_origins+0x11d/0x170 mm/kmsan/kmsan.c:527
 __msan_memcpy+0x109/0x160 mm/kmsan/kmsan_instr.c:413
 __nla_put lib/nlattr.c:569 [inline]
 nla_put+0x276/0x340 lib/nlattr.c:627
 copy_to_user_policy_type net/xfrm/xfrm_user.c:1678 [inline]
 dump_one_policy+0xbe1/0x1090 net/xfrm/xfrm_user.c:1708
 xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013
 xfrm_dump_policy+0x1c0/0x2a0 net/xfrm/xfrm_user.c:1749
 netlink_dump+0x9b5/0x1550 net/netlink/af_netlink.c:2226
 __netlink_dump_start+0x1131/0x1270 net/netlink/af_netlink.c:2323
 netlink_dump_start include/linux/netlink.h:214 [inline]
 xfrm_user_rcv_msg+0x8a3/0x9b0 net/xfrm/xfrm_user.c:2577
 netlink_rcv_skb+0x37e/0x600 net/netlink/af_netlink.c:2448
 xfrm_netlink_rcv+0xb2/0xf0 net/xfrm/xfrm_user.c:2598
 netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
 netlink_unicast+0x1680/0x1750 net/netlink/af_netlink.c:1336
 netlink_sendmsg+0x104f/0x1350 net/netlink/af_netlink.c:1901
 sock_sendmsg_nosec net/socket.c:629 [inline]
 sock_sendmsg net/socket.c:639 [inline]
 ___sys_sendmsg+0xec8/0x1320 net/socket.c:2117
 __sys_sendmsg net/socket.c:2155 [inline]
 __do_sys_sendmsg net/socket.c:2164 [inline]
 __se_sys_sendmsg net/socket.c:2162 [inline]
 __x64_sys_sendmsg+0x331/0x460 net/socket.c:2162
 do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
Local variable description: ----upt.i@dump_one_policy
Variable was created at:
 dump_one_policy+0x78/0x1090 net/xfrm/xfrm_user.c:1689
 xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013

Byte 130 of 137 is uninitialized
Memory access starts at ffff88019550407f

Fixes: c0144beaeca42 ("[XFRM] netlink: Use nla_put()/NLA_PUT() variantes")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 080035f056d9..1e50b70ad668 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1671,9 +1671,11 @@ static inline unsigned int userpolicy_type_attrsize(void)
 #ifdef CONFIG_XFRM_SUB_POLICY
 static int copy_to_user_policy_type(u8 type, struct sk_buff *skb)
 {
-	struct xfrm_userpolicy_type upt = {
-		.type = type,
-	};
+	struct xfrm_userpolicy_type upt;
+
+	/* Sadly there are two holes in struct xfrm_userpolicy_type */
+	memset(&upt, 0, sizeof(upt));
+	upt.type = type;
 
 	return nla_put(skb, XFRMA_POLICY_TYPE, sizeof(upt), &upt);
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] xfrm: fix missing dst_release() after policy blocking lbcast and multicast
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
  2018-07-27  6:51 ` [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit Steffen Klassert
  2018-07-27  6:51 ` [PATCH 2/5] xfrm_user: prevent leaking 2 bytes of kernel memory Steffen Klassert
@ 2018-07-27  6:51 ` Steffen Klassert
  2018-07-27  6:51 ` [PATCH 4/5] xfrm: free skb if nlsk pointer is NULL Steffen Klassert
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Tommi Rantala <tommi.t.rantala@nokia.com>

Fix missing dst_release() when local broadcast or multicast traffic is
xfrm policy blocked.

For IPv4 this results to dst leak: ip_route_output_flow() allocates
dst_entry via __ip_route_output_key() and passes it to
xfrm_lookup_route(). xfrm_lookup returns ERR_PTR(-EPERM) that is
propagated. The dst that was allocated is never released.

IPv4 local broadcast testcase:
 ping -b 192.168.1.255 &
 sleep 1
 ip xfrm policy add src 0.0.0.0/0 dst 192.168.1.255/32 dir out action block

IPv4 multicast testcase:
 ping 224.0.0.1 &
 sleep 1
 ip xfrm policy add src 0.0.0.0/0 dst 224.0.0.1/32 dir out action block

For IPv6 the missing dst_release() causes trouble e.g. when used in netns:
 ip netns add TEST
 ip netns exec TEST ip link set lo up
 ip link add dummy0 type dummy
 ip link set dev dummy0 netns TEST
 ip netns exec TEST ip addr add fd00::1111 dev dummy0
 ip netns exec TEST ip link set dummy0 up
 ip netns exec TEST ping -6 -c 5 ff02::1%dummy0 &
 sleep 1
 ip netns exec TEST ip xfrm policy add src ::/0 dst ff02::1 dir out action block
 wait
 ip netns del TEST

After netns deletion we see:
[  258.239097] unregister_netdevice: waiting for lo to become free. Usage count = 2
[  268.279061] unregister_netdevice: waiting for lo to become free. Usage count = 2
[  278.367018] unregister_netdevice: waiting for lo to become free. Usage count = 2
[  288.375259] unregister_netdevice: waiting for lo to become free. Usage count = 2

Fixes: ac37e2515c1a ("xfrm: release dst_orig in case of error in xfrm_lookup()")
Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5f48251c1319..7c5e8978aeaa 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2286,6 +2286,9 @@ struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry *dst_orig,
 	if (IS_ERR(dst) && PTR_ERR(dst) == -EREMOTE)
 		return make_blackhole(net, dst_orig->ops->family, dst_orig);
 
+	if (IS_ERR(dst))
+		dst_release(dst_orig);
+
 	return dst;
 }
 EXPORT_SYMBOL(xfrm_lookup_route);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] xfrm: free skb if nlsk pointer is NULL
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
                   ` (2 preceding siblings ...)
  2018-07-27  6:51 ` [PATCH 3/5] xfrm: fix missing dst_release() after policy blocking lbcast and multicast Steffen Klassert
@ 2018-07-27  6:51 ` Steffen Klassert
  2018-07-27  6:51 ` [PATCH 5/5] esp6: fix memleak on error path in esp6_input Steffen Klassert
  2018-07-27 16:20 ` pull request (net): ipsec 2018-07-27 David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Florian Westphal <fw@strlen.de>

nlmsg_multicast() always frees the skb, so in case we cannot call
it we must do that ourselves.

Fixes: 21ee543edc0dea ("xfrm: fix race between netns cleanup and state expire notification")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 1e50b70ad668..33878e6e0d0a 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1025,10 +1025,12 @@ static inline int xfrm_nlmsg_multicast(struct net *net, struct sk_buff *skb,
 {
 	struct sock *nlsk = rcu_dereference(net->xfrm.nlsk);
 
-	if (nlsk)
-		return nlmsg_multicast(nlsk, skb, pid, group, GFP_ATOMIC);
-	else
-		return -1;
+	if (!nlsk) {
+		kfree_skb(skb);
+		return -EPIPE;
+	}
+
+	return nlmsg_multicast(nlsk, skb, pid, group, GFP_ATOMIC);
 }
 
 static inline unsigned int xfrm_spdinfo_msgsize(void)
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] esp6: fix memleak on error path in esp6_input
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
                   ` (3 preceding siblings ...)
  2018-07-27  6:51 ` [PATCH 4/5] xfrm: free skb if nlsk pointer is NULL Steffen Klassert
@ 2018-07-27  6:51 ` Steffen Klassert
  2018-07-27 16:20 ` pull request (net): ipsec 2018-07-27 David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Steffen Klassert @ 2018-07-27  6:51 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Zhen Lei <thunder.leizhen@huawei.com>

This ought to be an omission in e6194923237 ("esp: Fix memleaks on error
paths."). The memleak on error path in esp6_input is similar to esp_input
of esp4.

Fixes: e6194923237 ("esp: Fix memleaks on error paths.")
Fixes: 3f29770723f ("ipsec: check return value of skb_to_sgvec always")
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/esp6.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 97513f35bcc5..88a7579c23bd 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -669,8 +669,10 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 
 	sg_init_table(sg, nfrags);
 	ret = skb_to_sgvec(skb, sg, 0, skb->len);
-	if (unlikely(ret < 0))
+	if (unlikely(ret < 0)) {
+		kfree(tmp);
 		goto out;
+	}
 
 	skb->ip_summed = CHECKSUM_NONE;
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: pull request (net): ipsec 2018-07-27
  2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
                   ` (4 preceding siblings ...)
  2018-07-27  6:51 ` [PATCH 5/5] esp6: fix memleak on error path in esp6_input Steffen Klassert
@ 2018-07-27 16:20 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2018-07-27 16:20 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 27 Jul 2018 08:51:49 +0200

> 1) Fix PMTU handling of vti6. We update the PMTU on
>    the xfrm dst_entry which is not cached anymore
>    after the flowchache removal. So update the
>    PMTU of the original dst_entry instead.
>    From Eyal Birger.
> 
> 2) Fix a leak of kernel memory to userspace.
>    From Eric Dumazet.
> 
> 3) Fix a possible dst_entry memleak in xfrm_lookup_route.
>    From Tommi Rantala.
> 
> 4) Fix a skb leak in case we can't call nlmsg_multicast
>    from xfrm_nlmsg_multicast. From Florian Westphal.
> 
> 5) Fix a leak of a temporary buffer in the error path of
>    esp6_input. From Zhen Lei.
> 
> Please pull or let me know if there are problems.

Pulled, thanks Steffen!

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-27 17:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27  6:51 pull request (net): ipsec 2018-07-27 Steffen Klassert
2018-07-27  6:51 ` [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit Steffen Klassert
2018-07-27  6:51 ` [PATCH 2/5] xfrm_user: prevent leaking 2 bytes of kernel memory Steffen Klassert
2018-07-27  6:51 ` [PATCH 3/5] xfrm: fix missing dst_release() after policy blocking lbcast and multicast Steffen Klassert
2018-07-27  6:51 ` [PATCH 4/5] xfrm: free skb if nlsk pointer is NULL Steffen Klassert
2018-07-27  6:51 ` [PATCH 5/5] esp6: fix memleak on error path in esp6_input Steffen Klassert
2018-07-27 16:20 ` pull request (net): ipsec 2018-07-27 David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.