* [PATCH 1/2] net: ipv6: keep sk status consistent after datagram connect failure
2020-12-30 19:33 [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit() Suren Baghdasaryan
@ 2020-12-30 19:33 ` Suren Baghdasaryan
2020-12-30 19:33 ` [PATCH 2/2] l2tp: fix races with ipv4-mapped ipv6 addresses Suren Baghdasaryan
2020-12-31 9:17 ` [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit() Greg KH
2 siblings, 0 replies; 5+ messages in thread
From: Suren Baghdasaryan @ 2020-12-30 19:33 UTC (permalink / raw)
To: surenb; +Cc: pabeni, davem, kuba, yoshfuji, kuznet, gregkh, stable
From: Paolo Abeni <pabeni@redhat.com>
commit 2f987a76a97773beafbc615b9c4d8fe79129a7f4 upstream
On unsuccesful ip6_datagram_connect(), if the failure is caused by
ip6_datagram_dst_update(), the sk peer information are cleared, but
the sk->sk_state is preserved.
If the socket was already in an established status, the overall sk
status is inconsistent and fouls later checks in datagram code.
Fix this saving the old peer information and restoring them in
case of failure. This also aligns ipv6 datagram connect() behavior
with ipv4.
v1 -> v2:
- added missing Fixes tag
Fixes: 85cb73ff9b74 ("net: ipv6: reset daddr and dport in sk if connect() fails")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv6/datagram.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 58929622de0e..fa4987827cda 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -145,10 +145,12 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
struct sockaddr_in6 *usin = (struct sockaddr_in6 *) uaddr;
struct inet_sock *inet = inet_sk(sk);
struct ipv6_pinfo *np = inet6_sk(sk);
- struct in6_addr *daddr;
+ struct in6_addr *daddr, old_daddr;
+ __be32 fl6_flowlabel = 0;
+ __be32 old_fl6_flowlabel;
+ __be32 old_dport;
int addr_type;
int err;
- __be32 fl6_flowlabel = 0;
if (usin->sin6_family == AF_INET) {
if (__ipv6_only_sock(sk))
@@ -238,9 +240,13 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
}
}
+ /* save the current peer information before updating it */
+ old_daddr = sk->sk_v6_daddr;
+ old_fl6_flowlabel = np->flow_label;
+ old_dport = inet->inet_dport;
+
sk->sk_v6_daddr = *daddr;
np->flow_label = fl6_flowlabel;
-
inet->inet_dport = usin->sin6_port;
/*
@@ -249,8 +255,15 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
*/
err = ip6_datagram_dst_update(sk, true);
- if (err)
+ if (err) {
+ /* Restore the socket peer info, to keep it consistent with
+ * the old socket state
+ */
+ sk->sk_v6_daddr = old_daddr;
+ np->flow_label = old_fl6_flowlabel;
+ inet->inet_dport = old_dport;
goto out;
+ }
sk->sk_state = TCP_ESTABLISHED;
sk_set_txhash(sk);
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] l2tp: fix races with ipv4-mapped ipv6 addresses
2020-12-30 19:33 [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit() Suren Baghdasaryan
2020-12-30 19:33 ` [PATCH 1/2] net: ipv6: keep sk status consistent after datagram connect failure Suren Baghdasaryan
@ 2020-12-30 19:33 ` Suren Baghdasaryan
2020-12-31 9:17 ` [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit() Greg KH
2 siblings, 0 replies; 5+ messages in thread
From: Suren Baghdasaryan @ 2020-12-30 19:33 UTC (permalink / raw)
To: surenb
Cc: pabeni, davem, kuba, yoshfuji, kuznet, gregkh, stable,
syzbot+92fa328176eb07e4ac1a
From: Paolo Abeni <pabeni@redhat.com>
commit b954f94023dcc61388c8384f0f14eb8e42c863c5 upstream
The l2tp_tunnel_create() function checks for v4mapped ipv6
sockets and cache that flag, so that l2tp core code can
reusing it at xmit time.
If the socket is provided by the userspace, the connection
status of the tunnel sockets can change between the tunnel
creation and the xmit call, so that syzbot is able to
trigger the following splat:
BUG: KASAN: use-after-free in ip6_dst_idev include/net/ip6_fib.h:192
[inline]
BUG: KASAN: use-after-free in ip6_xmit+0x1f76/0x2260
net/ipv6/ip6_output.c:264
Read of size 8 at addr ffff8801bd949318 by task syz-executor4/23448
CPU: 0 PID: 23448 Comm: syz-executor4 Not tainted 4.16.0-rc4+ #65
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x24d lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report+0x23c/0x360 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
ip6_dst_idev include/net/ip6_fib.h:192 [inline]
ip6_xmit+0x1f76/0x2260 net/ipv6/ip6_output.c:264
inet6_csk_xmit+0x2fc/0x580 net/ipv6/inet6_connection_sock.c:139
l2tp_xmit_core net/l2tp/l2tp_core.c:1053 [inline]
l2tp_xmit_skb+0x105f/0x1410 net/l2tp/l2tp_core.c:1148
pppol2tp_sendmsg+0x470/0x670 net/l2tp/l2tp_ppp.c:341
sock_sendmsg_nosec net/socket.c:630 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:640
___sys_sendmsg+0x767/0x8b0 net/socket.c:2046
__sys_sendmsg+0xe5/0x210 net/socket.c:2080
SYSC_sendmsg net/socket.c:2091 [inline]
SyS_sendmsg+0x2d/0x50 net/socket.c:2087
do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x453e69
RSP: 002b:00007f819593cc68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f819593d6d4 RCX: 0000000000453e69
RDX: 0000000000000081 RSI: 000000002037ffc8 RDI: 0000000000000004
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000000004c3 R14: 00000000006f72e8 R15: 0000000000000000
This change addresses the issues:
* explicitly checking for TCP_ESTABLISHED for user space provided sockets
* dropping the v4mapped flag usage - it can become outdated - and
explicitly invoking ipv6_addr_v4mapped() instead
The issue is apparently there since ancient times.
v1 -> v2: (many thanks to Guillaume)
- with csum issue introduced in v1
- replace pr_err with pr_debug
- fix build issue with IPV6 disabled
- move l2tp_sk_is_v4mapped in l2tp_core.c
v2 -> v3:
- don't update inet_daddr for v4mapped address, unneeded
- drop rendundant check at creation time
Reported-and-tested-by: syzbot+92fa328176eb07e4ac1a@syzkaller.appspotmail.com
Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/l2tp/l2tp_core.c | 38 ++++++++++++++++++--------------------
net/l2tp/l2tp_core.h | 3 ---
2 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index a8f575bf9b7c..15f8bd0364c2 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -112,6 +112,13 @@ struct l2tp_net {
spinlock_t l2tp_session_hlist_lock;
};
+#if IS_ENABLED(CONFIG_IPV6)
+static bool l2tp_sk_is_v6(struct sock *sk)
+{
+ return sk->sk_family == PF_INET6 &&
+ !ipv6_addr_v4mapped(&sk->sk_v6_daddr);
+}
+#endif
static inline struct l2tp_tunnel *l2tp_tunnel(struct sock *sk)
{
@@ -1136,7 +1143,7 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
skb->ignore_df = 1;
skb_dst_drop(skb);
#if IS_ENABLED(CONFIG_IPV6)
- if (tunnel->sock->sk_family == PF_INET6 && !tunnel->v4mapped)
+ if (l2tp_sk_is_v6(tunnel->sock))
error = inet6_csk_xmit(tunnel->sock, skb, NULL);
else
#endif
@@ -1199,6 +1206,15 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
goto out_unlock;
}
+ /* The user-space may change the connection status for the user-space
+ * provided socket at run time: we must check it under the socket lock
+ */
+ if (tunnel->fd >= 0 && sk->sk_state != TCP_ESTABLISHED) {
+ kfree_skb(skb);
+ ret = NET_XMIT_DROP;
+ goto out_unlock;
+ }
+
inet = inet_sk(sk);
fl = &inet->cork.fl;
switch (tunnel->encap) {
@@ -1214,7 +1230,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
/* Calculate UDP checksum if configured to do so */
#if IS_ENABLED(CONFIG_IPV6)
- if (sk->sk_family == PF_INET6 && !tunnel->v4mapped)
+ if (l2tp_sk_is_v6(sk))
udp6_set_csum(udp_get_no_check6_tx(sk),
skb, &inet6_sk(sk)->saddr,
&sk->sk_v6_daddr, udp_len);
@@ -1620,24 +1636,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
if (cfg != NULL)
tunnel->debug = cfg->debug;
-#if IS_ENABLED(CONFIG_IPV6)
- if (sk->sk_family == PF_INET6) {
- struct ipv6_pinfo *np = inet6_sk(sk);
-
- if (ipv6_addr_v4mapped(&np->saddr) &&
- ipv6_addr_v4mapped(&sk->sk_v6_daddr)) {
- struct inet_sock *inet = inet_sk(sk);
-
- tunnel->v4mapped = true;
- inet->inet_saddr = np->saddr.s6_addr32[3];
- inet->inet_rcv_saddr = sk->sk_v6_rcv_saddr.s6_addr32[3];
- inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
- } else {
- tunnel->v4mapped = false;
- }
- }
-#endif
-
/* Mark socket as an encapsulation socket. See net/ipv4/udp.c */
tunnel->encap = encap;
if (encap == L2TP_ENCAPTYPE_UDP) {
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 2b9b6fb67ae9..57c1810f1a32 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -191,9 +191,6 @@ struct l2tp_tunnel {
struct sock *sock; /* Parent socket */
int fd; /* Parent fd, if tunnel socket
* was created by userspace */
-#if IS_ENABLED(CONFIG_IPV6)
- bool v4mapped;
-#endif
struct work_struct del_work;
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit()
2020-12-30 19:33 [PATCH 0/2] backports for slab-out-of-bounds issue in ip6_xmit() Suren Baghdasaryan
2020-12-30 19:33 ` [PATCH 1/2] net: ipv6: keep sk status consistent after datagram connect failure Suren Baghdasaryan
2020-12-30 19:33 ` [PATCH 2/2] l2tp: fix races with ipv4-mapped ipv6 addresses Suren Baghdasaryan
@ 2020-12-31 9:17 ` Greg KH
2021-01-05 16:38 ` Suren Baghdasaryan
2 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2020-12-31 9:17 UTC (permalink / raw)
To: Suren Baghdasaryan; +Cc: pabeni, davem, kuba, yoshfuji, kuznet, stable
On Wed, Dec 30, 2020 at 11:33:21AM -0800, Suren Baghdasaryan wrote:
> We received a slab-out-of-bounds report in ip6_xmit() for KASAN build on 4.9
> kernel. The patches that fix this issue have been backported to to stable 4.14
> and one of them even to 3.16 but 4.9 stable branch does not include them.
> Backport to linux-4.9.y required trivial merge conflict resolution. They
> cleanly apply to linux-stable linux-4.9.y branch tagged v4.9.249.
>
> Paolo Abeni (2):
> net: ipv6: keep sk status consistent after datagram connect failure
> l2tp: fix races with ipv4-mapped ipv6 addresses
>
> net/ipv6/datagram.c | 21 +++++++++++++++++----
> net/l2tp/l2tp_core.c | 38 ++++++++++++++++++--------------------
> net/l2tp/l2tp_core.h | 3 ---
> 3 files changed, 35 insertions(+), 27 deletions(-)
Nit, you forgot to sign-off on these patches that you forwarded on.
Next time remember to do that as you did "touch" them :)
I'll go queue them up now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread