Skip to content

Commit b0519de

Browse files
Florian Westphaldavem330
authored andcommitted
mptcp: fix use-after-free for ipv6
Turns out that when we accept a new subflow, the newly created inet_sk(tcp_sk)->pinet6 points at the ipv6_pinfo structure of the listener socket. This wasn't caught by the selftest because it closes the accepted fd before the listening one. adding a close(listenfd) after accept returns is enough: BUG: KASAN: use-after-free in inet6_getname+0x6ba/0x790 Read of size 1 at addr ffff88810e310866 by task mptcp_connect/2518 Call Trace: inet6_getname+0x6ba/0x790 __sys_getpeername+0x10b/0x250 __x64_sys_getpeername+0x6f/0xb0 also alter test program to exercise this. Reported-by: Christoph Paasch <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 0202d29 commit b0519de

File tree

2 files changed

+42
-3
lines changed

2 files changed

+42
-3
lines changed

net/mptcp/protocol.c

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@
2424

2525
#define MPTCP_SAME_STATE TCP_MAX_STATES
2626

27+
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
28+
struct mptcp6_sock {
29+
struct mptcp_sock msk;
30+
struct ipv6_pinfo np;
31+
};
32+
#endif
33+
2734
/* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
2835
* completed yet or has failed, return the subflow socket.
2936
* Otherwise return NULL.
@@ -627,6 +634,30 @@ static void mptcp_copy_inaddrs(struct sock *msk, const struct sock *ssk)
627634
inet_sk(msk)->inet_rcv_saddr = inet_sk(ssk)->inet_rcv_saddr;
628635
}
629636

637+
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
638+
static struct ipv6_pinfo *mptcp_inet6_sk(const struct sock *sk)
639+
{
640+
unsigned int offset = sizeof(struct mptcp6_sock) - sizeof(struct ipv6_pinfo);
641+
642+
return (struct ipv6_pinfo *)(((u8 *)sk) + offset);
643+
}
644+
#endif
645+
646+
struct sock *mptcp_sk_clone_lock(const struct sock *sk)
647+
{
648+
struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
649+
650+
if (!nsk)
651+
return NULL;
652+
653+
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
654+
if (nsk->sk_family == AF_INET6)
655+
inet_sk(nsk)->pinet6 = mptcp_inet6_sk(nsk);
656+
#endif
657+
658+
return nsk;
659+
}
660+
630661
static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
631662
bool kern)
632663
{
@@ -657,7 +688,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
657688
lock_sock(sk);
658689

659690
local_bh_disable();
660-
new_mptcp_sock = sk_clone_lock(sk, GFP_ATOMIC);
691+
new_mptcp_sock = mptcp_sk_clone_lock(sk);
661692
if (!new_mptcp_sock) {
662693
*err = -ENOBUFS;
663694
local_bh_enable();
@@ -1206,8 +1237,7 @@ int mptcp_proto_v6_init(void)
12061237
strcpy(mptcp_v6_prot.name, "MPTCPv6");
12071238
mptcp_v6_prot.slab = NULL;
12081239
mptcp_v6_prot.destroy = mptcp_v6_destroy;
1209-
mptcp_v6_prot.obj_size = sizeof(struct mptcp_sock) +
1210-
sizeof(struct ipv6_pinfo);
1240+
mptcp_v6_prot.obj_size = sizeof(struct mptcp6_sock);
12111241

12121242
err = proto_register(&mptcp_v6_prot, 1);
12131243
if (err)

tools/testing/selftests/net/mptcp/mptcp_connect.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,14 @@ static void check_getpeername_connect(int fd)
634634
cfg_host, a, cfg_port, b);
635635
}
636636

637+
static void maybe_close(int fd)
638+
{
639+
unsigned int r = rand();
640+
641+
if (r & 1)
642+
close(fd);
643+
}
644+
637645
int main_loop_s(int listensock)
638646
{
639647
struct sockaddr_storage ss;
@@ -657,6 +665,7 @@ int main_loop_s(int listensock)
657665
salen = sizeof(ss);
658666
remotesock = accept(listensock, (struct sockaddr *)&ss, &salen);
659667
if (remotesock >= 0) {
668+
maybe_close(listensock);
660669
check_sockaddr(pf, &ss, salen);
661670
check_getpeername(remotesock, &ss, salen);
662671

0 commit comments

Comments
 (0)