* Re: [MPTCP] [PATCH] mptcp: fix oops on accept
@ 2019-04-24 8:28 Paolo Abeni
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2019-04-24 8:28 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]
On Tue, 2019-04-23 at 17:17 -0700, Mat Martineau wrote:
> On Thu, 18 Apr 2019, Paolo Abeni wrote:
>
> > while running the self-test on a multi core VM, with a standard (non
> > debug) kernel config, I hit the following oops:
> >
> > [ 234.587877] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> > [ 234.591567] #PF error: [normal kernel read fault]
> > [ 234.593862] PGD 800000013a8ec067 P4D 800000013a8ec067 PUD 13a2ad067 PMD 0
> > [ 234.596616] Oops: 0000 [#1] SMP PTI
> > [ 234.597173] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.1.0-rc4.mptcp_xmit_07723d4+ #2139
> > [ 234.598363] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
> > [ 234.599847] RIP: 0010:mptcp_finish_connect+0x2a/0x160
>
> ...
>
> > The problem is in mptcp_accept(): the newly created subflow is
> > already added to the mptcp subflow list, but conn_finished is not set.
> >
> > On next rx dst cache update we enter subflow_finish_connect()/mptcp_finish_connect()
> > and hit the crash due to NULL msk->subflow de-referncing.
> >
> > Fix the above properly initializing the subflow at accept() time.
>
> Peter and I have been looking in to this crash. It could be due to a
> use-after-free bug, since conn_finished isn't expected to be used on the
> accepting socket. The symptoms are similar to KASAN report I was seeing on
> my multicore VM.
Uhmm... possibly there are some additional races, but the oops I'm
seeing is pretty much deterministic, not due to a race.
subflow_finish_connect() is invoked via:
tcp_rcv_established() -> inet_csk(sk)->icsk_af_ops->sk_rx_dst_set() ->
subflow_finish_connect()
every time that the rx cache dst expires on the subflow socket created
by the accept() syscall.
When the relevant mptcp socket and the subflow are created at accept()
time - in mptcp_accept(), we have:
msk->subflow = NULL;
// ...
subflow->conn = new_mptcp_sock->sk;
// subflow->conn_finished is memset to 0 at allocation time
so mptcp_finish_connect() is invoked. mptcp_finish_connect()
unconditionally dereference mptcp_sk(subflow->conn)->subflow, and that
causes the reported oops.
So I think setting 'msk->conn_finished = 1' in mptcp_accept() is
required to properly initialize the mptcp socket created on accept().
Cheers,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [MPTCP] [PATCH] mptcp: fix oops on accept
@ 2019-04-24 21:17 Peter Krystad
0 siblings, 0 replies; 4+ messages in thread
From: Peter Krystad @ 2019-04-24 21:17 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4331 bytes --]
On Thu, 2019-04-18 at 15:25 +0200, Paolo Abeni wrote:
> while running the self-test on a multi core VM, with a standard (non
> debug) kernel config, I hit the following oops:
>
> [ 234.587877] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> [ 234.591567] #PF error: [normal kernel read fault]
> [ 234.593862] PGD 800000013a8ec067 P4D 800000013a8ec067 PUD 13a2ad067 PMD 0
> [ 234.596616] Oops: 0000 [#1] SMP PTI
> [ 234.597173] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.1.0-rc4.mptcp_xmit_07723d4+ #2139
> [ 234.598363] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
> [ 234.599847] RIP: 0010:mptcp_finish_connect+0x2a/0x160
> [ 234.600464] Code: 0f 1f 44 00 00 41 54 41 89 f4 55 53 48 89 fb 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 31 c0 48 8b 87 80 05 00 00 <48> 8b 40 20 48 8b a8 88 04 00 00 0f 1f 44 00 00 45 85 e4 0f 84 ad
> [ 234.602646] RSP: 0018:ffffa014fbb03c78 EFLAGS: 00010246
> [ 234.603307] RAX: 0000000000000000 RBX: ffffa014fac92280 RCX: 0000000000000001
> [ 234.604158] RDX: ffffa014fa00c480 RSI: 0000000000000001 RDI: ffffa014fac92280
> [ 234.605010] RBP: ffffa014f9a48000 R08: ffffa014fa00c4c0 R09: 0000000000000000
> [ 234.605861] R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000001
> [ 234.606701] R13: ffffa014f6c99424 R14: 0000000000000014 R15: ffffa014f6c99410
> [ 234.607555] FS: 0000000000000000(0000) GS:ffffa014fbb00000(0000) knlGS:0000000000000000
> [ 234.608498] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 234.609110] CR2: 0000000000000020 CR3: 0000000137e22004 CR4: 00000000001606e0
> [ 234.609863] Call Trace:
> [ 234.610152] <IRQ>
> [ 234.610407] subflow_finish_connect+0x3d/0xc0
> [ 234.610883] tcp_rcv_established+0x641/0x660
> [ 234.611353] ? tcp_v4_inbound_md5_hash+0x64/0x1d0
> [ 234.611862] tcp_v4_do_rcv+0xf4/0x1e0
> [ 234.612269] tcp_v4_rcv+0x99c/0xaa0
> [ 234.612665] ip_protocol_deliver_rcu+0x4b/0x180
> [ 234.613159] ip_local_deliver_finish+0x40/0x50
> [ 234.613647] ip_local_deliver+0x6b/0xe0
> [ 234.614071] ? ip_rcv_finish+0x62/0xa0
> [ 234.614495] ip_rcv+0x52/0xd0
> [ 234.614841] __netif_receive_skb_one_core+0x52/0x70
> [ 234.615403] process_backlog+0x97/0x130
> [ 234.615897] net_rx_action+0x299/0x3d0
> [ 234.616319] __do_softirq+0xd8/0x2b7
> [ 234.616730] irq_exit+0xd5/0xe0
> [ 234.617089] smp_apic_timer_interrupt+0x74/0x140
> [ 234.617933] apic_timer_interrupt+0xf/0x20
> [ 234.618623] </IRQ>
> [ 234.619003] RIP: 0010:default_idle+0x17/0x140
> [ 234.619635] Code: ff 4c 89 e7 e8 7a ca c9 ff fb eb 98 90 90 90 90 90 90 90 0f 1f 44 00 00 41 54 55 53 65 8b 2d 00 95 f6 56 0f 1f 44 00 00 fb f4 <65> 8b 2d f2 94 f6 56 0f 1f 44 00 00 5b 5d 41 5c c3 65 8b 05 e1 94
>
> The problem is in mptcp_accept(): the newly created subflow is
> already added to the mptcp subflow list, but conn_finished is not set.
>
> On next rx dst cache update we enter subflow_finish_connect()/mptcp_finish_connect()
> and hit the crash due to NULL msk->subflow de-referncing.
>
> Fix the above properly initializing the subflow at accept() time.
>
> Fixes: a36080381c36 ("mptcp: Create SUBFLOW socket for incoming connections")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
>
> ---
> Note: I'm ok with swashing this change in the commit referenced by the
> 'fixes' tag.
> ---
> net/mptcp/protocol.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 55a39ec748b5..fcfb74818c30 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -639,6 +639,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
> subflow->map_subflow_seq = 1;
> subflow->rel_write_seq = 1;
> subflow->conn = new_mptcp_sock->sk;
> + subflow->conn_finished = 1;
> } else {
> msk->subflow = new_sock;
> }
Paolo -
Your finding looks good to me, thanks. I'll squash it into the "mptcp:
Create SUBFLOW socket for incoming connections" commit. Note that this
will require moving the introduction of the conn_finished field up into
to "outgoing connection" commit, which is probably where it belongs
anyway.
Peter.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [MPTCP] [PATCH] mptcp: fix oops on accept
@ 2019-04-24 0:17 Mat Martineau
0 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2019-04-24 0:17 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 2490 bytes --]
On Thu, 18 Apr 2019, Paolo Abeni wrote:
> while running the self-test on a multi core VM, with a standard (non
> debug) kernel config, I hit the following oops:
>
> [ 234.587877] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> [ 234.591567] #PF error: [normal kernel read fault]
> [ 234.593862] PGD 800000013a8ec067 P4D 800000013a8ec067 PUD 13a2ad067 PMD 0
> [ 234.596616] Oops: 0000 [#1] SMP PTI
> [ 234.597173] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.1.0-rc4.mptcp_xmit_07723d4+ #2139
> [ 234.598363] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
> [ 234.599847] RIP: 0010:mptcp_finish_connect+0x2a/0x160
...
>
> The problem is in mptcp_accept(): the newly created subflow is
> already added to the mptcp subflow list, but conn_finished is not set.
>
> On next rx dst cache update we enter subflow_finish_connect()/mptcp_finish_connect()
> and hit the crash due to NULL msk->subflow de-referncing.
>
> Fix the above properly initializing the subflow at accept() time.
Peter and I have been looking in to this crash. It could be due to a
use-after-free bug, since conn_finished isn't expected to be used on the
accepting socket. The symptoms are similar to KASAN report I was seeing on
my multicore VM.
I'm working on a full fix, but you might see if setting
newsk->icsk_ulp_ops and newsk->icsk_ulp_data to NULL in
inet_csk_clone_lock() prevents the crash.
Mat
>
> Fixes: a36080381c36 ("mptcp: Create SUBFLOW socket for incoming connections")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
>
> ---
> Note: I'm ok with swashing this change in the commit referenced by the
> 'fixes' tag.
> ---
> net/mptcp/protocol.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 55a39ec748b5..fcfb74818c30 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -639,6 +639,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
> subflow->map_subflow_seq = 1;
> subflow->rel_write_seq = 1;
> subflow->conn = new_mptcp_sock->sk;
> + subflow->conn_finished = 1;
> } else {
> msk->subflow = new_sock;
> }
> --
> 2.20.1
>
> _______________________________________________
> mptcp mailing list
> mptcp(a)lists.01.org
> https://lists.01.org/mailman/listinfo/mptcp
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] [PATCH] mptcp: fix oops on accept
@ 2019-04-18 13:25 Paolo Abeni
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2019-04-18 13:25 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3821 bytes --]
while running the self-test on a multi core VM, with a standard (non
debug) kernel config, I hit the following oops:
[ 234.587877] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
[ 234.591567] #PF error: [normal kernel read fault]
[ 234.593862] PGD 800000013a8ec067 P4D 800000013a8ec067 PUD 13a2ad067 PMD 0
[ 234.596616] Oops: 0000 [#1] SMP PTI
[ 234.597173] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.1.0-rc4.mptcp_xmit_07723d4+ #2139
[ 234.598363] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
[ 234.599847] RIP: 0010:mptcp_finish_connect+0x2a/0x160
[ 234.600464] Code: 0f 1f 44 00 00 41 54 41 89 f4 55 53 48 89 fb 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 31 c0 48 8b 87 80 05 00 00 <48> 8b 40 20 48 8b a8 88 04 00 00 0f 1f 44 00 00 45 85 e4 0f 84 ad
[ 234.602646] RSP: 0018:ffffa014fbb03c78 EFLAGS: 00010246
[ 234.603307] RAX: 0000000000000000 RBX: ffffa014fac92280 RCX: 0000000000000001
[ 234.604158] RDX: ffffa014fa00c480 RSI: 0000000000000001 RDI: ffffa014fac92280
[ 234.605010] RBP: ffffa014f9a48000 R08: ffffa014fa00c4c0 R09: 0000000000000000
[ 234.605861] R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000001
[ 234.606701] R13: ffffa014f6c99424 R14: 0000000000000014 R15: ffffa014f6c99410
[ 234.607555] FS: 0000000000000000(0000) GS:ffffa014fbb00000(0000) knlGS:0000000000000000
[ 234.608498] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 234.609110] CR2: 0000000000000020 CR3: 0000000137e22004 CR4: 00000000001606e0
[ 234.609863] Call Trace:
[ 234.610152] <IRQ>
[ 234.610407] subflow_finish_connect+0x3d/0xc0
[ 234.610883] tcp_rcv_established+0x641/0x660
[ 234.611353] ? tcp_v4_inbound_md5_hash+0x64/0x1d0
[ 234.611862] tcp_v4_do_rcv+0xf4/0x1e0
[ 234.612269] tcp_v4_rcv+0x99c/0xaa0
[ 234.612665] ip_protocol_deliver_rcu+0x4b/0x180
[ 234.613159] ip_local_deliver_finish+0x40/0x50
[ 234.613647] ip_local_deliver+0x6b/0xe0
[ 234.614071] ? ip_rcv_finish+0x62/0xa0
[ 234.614495] ip_rcv+0x52/0xd0
[ 234.614841] __netif_receive_skb_one_core+0x52/0x70
[ 234.615403] process_backlog+0x97/0x130
[ 234.615897] net_rx_action+0x299/0x3d0
[ 234.616319] __do_softirq+0xd8/0x2b7
[ 234.616730] irq_exit+0xd5/0xe0
[ 234.617089] smp_apic_timer_interrupt+0x74/0x140
[ 234.617933] apic_timer_interrupt+0xf/0x20
[ 234.618623] </IRQ>
[ 234.619003] RIP: 0010:default_idle+0x17/0x140
[ 234.619635] Code: ff 4c 89 e7 e8 7a ca c9 ff fb eb 98 90 90 90 90 90 90 90 0f 1f 44 00 00 41 54 55 53 65 8b 2d 00 95 f6 56 0f 1f 44 00 00 fb f4 <65> 8b 2d f2 94 f6 56 0f 1f 44 00 00 5b 5d 41 5c c3 65 8b 05 e1 94
The problem is in mptcp_accept(): the newly created subflow is
already added to the mptcp subflow list, but conn_finished is not set.
On next rx dst cache update we enter subflow_finish_connect()/mptcp_finish_connect()
and hit the crash due to NULL msk->subflow de-referncing.
Fix the above properly initializing the subflow at accept() time.
Fixes: a36080381c36 ("mptcp: Create SUBFLOW socket for incoming connections")
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
Note: I'm ok with swashing this change in the commit referenced by the
'fixes' tag.
---
net/mptcp/protocol.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 55a39ec748b5..fcfb74818c30 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -639,6 +639,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
subflow->map_subflow_seq = 1;
subflow->rel_write_seq = 1;
subflow->conn = new_mptcp_sock->sk;
+ subflow->conn_finished = 1;
} else {
msk->subflow = new_sock;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-24 21:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 8:28 [MPTCP] [PATCH] mptcp: fix oops on accept Paolo Abeni
-- strict thread matches above, loose matches on Subject: below --
2019-04-24 21:17 Peter Krystad
2019-04-24 0:17 Mat Martineau
2019-04-18 13:25 Paolo Abeni
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.