From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2012-09-04 17:11:00
On 09/04/2012 04:04 PM, Fengguang Wu wrote:
FYI, another kconfig triggering a slightly different oops on tree
git://gitorious.org/linux-can/linux-can-next led-trigger
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Marc
From: Eric W. Biederman <hidden> Date: 2012-09-04 20:32:29
Marc Kleine-Budde [off-list ref] writes:
On 09/04/2012 04:04 PM, Fengguang Wu wrote:
quoted
FYI, another kconfig triggering a slightly different oops on tree
git://gitorious.org/linux-can/linux-can-next led-trigger
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
This most definitely looks like a memory stomp somewhere.
sk->inet_sk->inet_opt has a bad value.
I am puzzled though what are we doing with both ipv4 and ipv6 release
state doing on the same socket path? Is this some crazy ipv6 socket
doing sctp with only ipv4 addresses?
Eric
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2012-09-04 20:42:20
On 09/04/2012 10:32 PM, Eric W. Biederman wrote:
quoted
quoted
FYI, another kconfig triggering a slightly different oops on tree
git://gitorious.org/linux-can/linux-can-next led-trigger
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
This most definitely looks like a memory stomp somewhere.
sk->inet_sk->inet_opt has a bad value.
I am puzzled though what are we doing with both ipv4 and ipv6 release
state doing on the same socket path? Is this some crazy ipv6 socket
doing sctp with only ipv4 addresses?
It's Wu's testcase, can you show us the code?
Eric, in case you haven't seen, this is another oops, from a slightly
different tree (a handfull of different CAN patches).
Wu is running a bisect, let's hope that gives us a result.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
On Tue, Sep 04, 2012 at 01:32:21PM -0700, Eric W. Biederman wrote:
Marc Kleine-Budde [off-list ref] writes:
quoted
On 09/04/2012 04:04 PM, Fengguang Wu wrote:
quoted
FYI, another kconfig triggering a slightly different oops on tree
git://gitorious.org/linux-can/linux-can-next led-trigger
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
You are both right. The bad commit turns out to be one of:
1bed966cc3bd4042110129f0fc51aeeb59c5b200 Merge branch 'tcp_fastopen_server'
168a8f58059a22feb9e9a2dcc1b8053dbbbc12ef tcp: TCP Fast Open Server - main code path
8336886f786fdacbc19b719c1f7ea91eb70706d4 tcp: TCP Fast Open Server - support TFO listeners
Thanks,
Fengguang
This most definitely looks like a memory stomp somewhere.
sk->inet_sk->inet_opt has a bad value.
I am puzzled though what are we doing with both ipv4 and ipv6 release
state doing on the same socket path? Is this some crazy ipv6 socket
doing sctp with only ipv4 addresses?
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2012-09-05 15:01:15
On 09/05/2012 04:55 PM, Fengguang Wu wrote:
quoted
quoted
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
You are both right. The bad commit turns out to be one of:
1bed966cc3bd4042110129f0fc51aeeb59c5b200 Merge branch 'tcp_fastopen_server'
168a8f58059a22feb9e9a2dcc1b8053dbbbc12ef tcp: TCP Fast Open Server - main code path
8336886f786fdacbc19b719c1f7ea91eb70706d4 tcp: TCP Fast Open Server - support TFO listeners
Thanks,
Fengguang
Thanks for your work Fengguang.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
From: Eric Dumazet <hidden> Date: 2012-09-05 15:30:52
On Wed, 2012-09-05 at 17:01 +0200, Marc Kleine-Budde wrote:
On 09/05/2012 04:55 PM, Fengguang Wu wrote:
quoted
quoted
quoted
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
You are both right. The bad commit turns out to be one of:
1bed966cc3bd4042110129f0fc51aeeb59c5b200 Merge branch 'tcp_fastopen_server'
168a8f58059a22feb9e9a2dcc1b8053dbbbc12ef tcp: TCP Fast Open Server - main code path
8336886f786fdacbc19b719c1f7ea91eb70706d4 tcp: TCP Fast Open Server - support TFO listeners
Thanks,
Fengguang
Thanks for your work Fengguang.
Marc
OK I have a good idea how to fix the bug, I will send a patch ASAP
Thanks
From: Eric Dumazet <hidden> Date: 2012-09-05 15:40:52
On Wed, 2012-09-05 at 17:30 +0200, Eric Dumazet wrote:
On Wed, 2012-09-05 at 17:01 +0200, Marc Kleine-Budde wrote:
quoted
On 09/05/2012 04:55 PM, Fengguang Wu wrote:
quoted
quoted
quoted
This in turn means the problem doesn't come from the CAN patches, as
both trees have different CAN patches. I'm adding Eric W. Biederman on
Cc as he contributed some sctp patches between v3.6 and net-next/master.
Anything is possible, but this seems unlikely as I don't think I touched
anything close to that part of the code.
You are both right. The bad commit turns out to be one of:
1bed966cc3bd4042110129f0fc51aeeb59c5b200 Merge branch 'tcp_fastopen_server'
168a8f58059a22feb9e9a2dcc1b8053dbbbc12ef tcp: TCP Fast Open Server - main code path
8336886f786fdacbc19b719c1f7ea91eb70706d4 tcp: TCP Fast Open Server - support TFO listeners
Thanks,
Fengguang
Thanks for your work Fengguang.
Marc
OK I have a good idea how to fix the bug, I will send a patch ASAP
Could you test the following patch please ?
(Not sure why sctp doesnt memset/bzero its whole socket by the way...)
Thanks
@@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)newsk=req->sk;sk_acceptq_removed(sk);-if(sk->sk_type==SOCK_STREAM&&queue->fastopenq!=NULL){+if(sk->sk_protocol==IPPROTO_TCP&&queue->fastopenq!=NULL){spin_lock_bh(&queue->fastopenq->lock);if(tcp_rsk(req)->listener){/* We are still waiting for the final ACK from 3WHS
On Wed, Sep 05, 2012 at 06:57:00PM +0200, Eric Dumazet wrote:
On Wed, 2012-09-05 at 17:40 +0200, Eric Dumazet wrote:
quoted
Could you test the following patch please ?
It works - no single error for 1000 boots!
btw, the first bad commit has been bisected to
commit 8336886f786fdacbc19b719c1f7ea91eb70706d4
Author: Jerry Chu [off-list ref]
Date: Fri Aug 31 12:29:12 2012 +0000
tcp: TCP Fast Open Server - support TFO listeners
quoted
(Not sure why sctp doesnt memset/bzero its whole socket by the way...)
Thanks
Here is a more complete patch, as there are three potential problems,
not only one :
@@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)newsk=req->sk;sk_acceptq_removed(sk);-if(sk->sk_type==SOCK_STREAM&&queue->fastopenq!=NULL){+if(sk->sk_protocol==IPPROTO_TCP&&queue->fastopenq!=NULL){spin_lock_bh(&queue->fastopenq->lock);if(tcp_rsk(req)->listener){/* We are still waiting for the final ACK from 3WHS
From: Jerry Chu <hidden> Date: 2012-09-05 23:07:09
On Wed, Sep 5, 2012 at 3:28 PM, Fengguang Wu [off-list ref] wrote:
On Wed, Sep 05, 2012 at 06:57:00PM +0200, Eric Dumazet wrote:
quoted
On Wed, 2012-09-05 at 17:40 +0200, Eric Dumazet wrote:
quoted
Could you test the following patch please ?
It works - no single error for 1000 boots!
Sorry for introducing the bug, one of the casualties dealing with code
that is shared
outside of TCP. I did spend some effort adding special checks but I was wrong in
assuming inet_create() will zero all the field including fastopenq
inside icsk_accept_queue
inside struct inet_connection_sock - although this is true for TCP and
DCCP, SCTP doesn't
have inet_connection_sock hence inet_csk(sk) is bogus.
Kudo to Eric for fixing it quickly before I got to it.
Jerry
btw, the first bad commit has been bisected to
commit 8336886f786fdacbc19b719c1f7ea91eb70706d4
Author: Jerry Chu [off-list ref]
Date: Fri Aug 31 12:29:12 2012 +0000
tcp: TCP Fast Open Server - support TFO listeners
quoted
quoted
(Not sure why sctp doesnt memset/bzero its whole socket by the way...)
Thanks
Here is a more complete patch, as there are three potential problems,
not only one :
@@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)newsk=req->sk;sk_acceptq_removed(sk);-if(sk->sk_type==SOCK_STREAM&&queue->fastopenq!=NULL){+if(sk->sk_protocol==IPPROTO_TCP&&queue->fastopenq!=NULL){spin_lock_bh(&queue->fastopenq->lock);if(tcp_rsk(req)->listener){/* We are still waiting for the final ACK from 3WHS
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)newsk=req->sk;sk_acceptq_removed(sk);-if(sk->sk_type==SOCK_STREAM&&queue->fastopenq!=NULL){+if(sk->sk_protocol==IPPROTO_TCP&&queue->fastopenq!=NULL){spin_lock_bh(&queue->fastopenq->lock);if(tcp_rsk(req)->listener){/* We are still waiting for the final ACK from 3WHS
On Thu, Sep 6, 2012 at 2:07 PM, Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
Fengguang Wu reported various panics and bisected to commit
8336886f786fdac (tcp: TCP Fast Open Server - support TFO listeners)
Fix this by making sure socket is a TCP socket before accessing TFO data
structures.
...
Reported-by: Fengguang Wu <redacted>
Tested-by: Fengguang Wu <redacted>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "H.K. Jerry Chu" <redacted>
@@ -314,7 +314,7 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)newsk=req->sk;sk_acceptq_removed(sk);-if(sk->sk_type==SOCK_STREAM&&queue->fastopenq!=NULL){+if(sk->sk_protocol==IPPROTO_TCP&&queue->fastopenq!=NULL){spin_lock_bh(&queue->fastopenq->lock);if(tcp_rsk(req)->listener){/* We are still waiting for the final ACK from 3WHS
On Thu, Sep 6, 2012 at 2:07 PM, Eric Dumazet [off-list ref] wrote:
quoted
From: Eric Dumazet <edumazet@google.com>
Fengguang Wu reported various panics and bisected to commit
8336886f786fdac (tcp: TCP Fast Open Server - support TFO listeners)
Fix this by making sure socket is a TCP socket before accessing TFO data
structures.
...
quoted
Reported-by: Fengguang Wu <redacted>
Tested-by: Fengguang Wu <redacted>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "H.K. Jerry Chu" <redacted>