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 <cpaasch@apple.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Florian Westphal 2020-02-06 00:39:37 +01:00 committed by David S. Miller
parent 0202d293c2
commit b0519de8b3
2 changed files with 42 additions and 3 deletions

View File

@ -24,6 +24,13 @@
#define MPTCP_SAME_STATE TCP_MAX_STATES #define MPTCP_SAME_STATE TCP_MAX_STATES
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
struct mptcp6_sock {
struct mptcp_sock msk;
struct ipv6_pinfo np;
};
#endif
/* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not /* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
* completed yet or has failed, return the subflow socket. * completed yet or has failed, return the subflow socket.
* Otherwise return NULL. * Otherwise return NULL.
@ -627,6 +634,30 @@ static void mptcp_copy_inaddrs(struct sock *msk, const struct sock *ssk)
inet_sk(msk)->inet_rcv_saddr = inet_sk(ssk)->inet_rcv_saddr; inet_sk(msk)->inet_rcv_saddr = inet_sk(ssk)->inet_rcv_saddr;
} }
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
static struct ipv6_pinfo *mptcp_inet6_sk(const struct sock *sk)
{
unsigned int offset = sizeof(struct mptcp6_sock) - sizeof(struct ipv6_pinfo);
return (struct ipv6_pinfo *)(((u8 *)sk) + offset);
}
#endif
struct sock *mptcp_sk_clone_lock(const struct sock *sk)
{
struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
if (!nsk)
return NULL;
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
if (nsk->sk_family == AF_INET6)
inet_sk(nsk)->pinet6 = mptcp_inet6_sk(nsk);
#endif
return nsk;
}
static struct sock *mptcp_accept(struct sock *sk, int flags, int *err, static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
bool kern) bool kern)
{ {
@ -657,7 +688,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
lock_sock(sk); lock_sock(sk);
local_bh_disable(); local_bh_disable();
new_mptcp_sock = sk_clone_lock(sk, GFP_ATOMIC); new_mptcp_sock = mptcp_sk_clone_lock(sk);
if (!new_mptcp_sock) { if (!new_mptcp_sock) {
*err = -ENOBUFS; *err = -ENOBUFS;
local_bh_enable(); local_bh_enable();
@ -1206,8 +1237,7 @@ int mptcp_proto_v6_init(void)
strcpy(mptcp_v6_prot.name, "MPTCPv6"); strcpy(mptcp_v6_prot.name, "MPTCPv6");
mptcp_v6_prot.slab = NULL; mptcp_v6_prot.slab = NULL;
mptcp_v6_prot.destroy = mptcp_v6_destroy; mptcp_v6_prot.destroy = mptcp_v6_destroy;
mptcp_v6_prot.obj_size = sizeof(struct mptcp_sock) + mptcp_v6_prot.obj_size = sizeof(struct mptcp6_sock);
sizeof(struct ipv6_pinfo);
err = proto_register(&mptcp_v6_prot, 1); err = proto_register(&mptcp_v6_prot, 1);
if (err) if (err)

View File

@ -634,6 +634,14 @@ static void check_getpeername_connect(int fd)
cfg_host, a, cfg_port, b); cfg_host, a, cfg_port, b);
} }
static void maybe_close(int fd)
{
unsigned int r = rand();
if (r & 1)
close(fd);
}
int main_loop_s(int listensock) int main_loop_s(int listensock)
{ {
struct sockaddr_storage ss; struct sockaddr_storage ss;
@ -657,6 +665,7 @@ int main_loop_s(int listensock)
salen = sizeof(ss); salen = sizeof(ss);
remotesock = accept(listensock, (struct sockaddr *)&ss, &salen); remotesock = accept(listensock, (struct sockaddr *)&ss, &salen);
if (remotesock >= 0) { if (remotesock >= 0) {
maybe_close(listensock);
check_sockaddr(pf, &ss, salen); check_sockaddr(pf, &ss, salen);
check_getpeername(remotesock, &ss, salen); check_getpeername(remotesock, &ss, salen);