| Message ID | 1345239312-19266-1-git-send-email-christoph.paasch@uclouvain.be |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Christoph Paasch <christoph.paasch@uclouvain.be> Date: Fri, 17 Aug 2012 23:35:12 +0200 > @@ -404,7 +404,7 @@ struct dst_entry *inet_csk_route_child_sock(struct sock *sk, > { > const struct inet_request_sock *ireq = inet_rsk(req); > struct inet_sock *newinet = inet_sk(newsk); > - struct ip_options_rcu *opt = ireq->opt; > + struct ip_options_rcu *opt = rcu_dereference(newinet->inet_opt); > struct net *net = sock_net(sk); We're not inside of a rcu_read_lock() protected section, so this access is not legitimate. If you enabled RCU lock debugging, you would have triggered a warning in the kernel log. -- 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
On Monday 20 August 2012 02:51:47 David Miller wrote: > From: Christoph Paasch <christoph.paasch@uclouvain.be> > Date: Fri, 17 Aug 2012 23:35:12 +0200 > > > @@ -404,7 +404,7 @@ struct dst_entry *inet_csk_route_child_sock(struct > > sock *sk,> > > { > > const struct inet_request_sock *ireq = inet_rsk(req); > > struct inet_sock *newinet = inet_sk(newsk); > > > > - struct ip_options_rcu *opt = ireq->opt; > > + struct ip_options_rcu *opt = rcu_dereference(newinet->inet_opt); > > > > struct net *net = sock_net(sk); > > We're not inside of a rcu_read_lock() protected section, so this access > is not legitimate. If you enabled RCU lock debugging, you would have > triggered a warning in the kernel log. Oh, sorry... I will resubmit a v2. Although, I enabled CONFIG_PROVE_RCU (and all other RCU-related debug I could find) and no warning was triggered. By the way, in dccp_v4_request_recv_sock() is the code: newinet->inet_opt = ireq->opt; Shouldn't this rather be an rcu_assign_pointer() ? And in cipso_v4_sock_delattr() I believe we should also rather access 'opt' instead of doing cipso_v4_delopt(&sk_inet->inet_opt) ? Christoph
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index db0cf17..f16bdc4 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -404,7 +404,7 @@ struct dst_entry *inet_csk_route_child_sock(struct sock *sk, { const struct inet_request_sock *ireq = inet_rsk(req); struct inet_sock *newinet = inet_sk(newsk); - struct ip_options_rcu *opt = ireq->opt; + struct ip_options_rcu *opt = rcu_dereference(newinet->inet_opt); struct net *net = sock_net(sk); struct flowi4 *fl4; struct rtable *rt;
Since 0e734419923bd ("ipv4: Use inet_csk_route_child_sock() in DCCP and TCP."), inet_csk_route_child_sock() is called instead of inet_csk_route_req(). However, after creating the child-sock in tcp/dccp_v4_syn_recv_sock(), ireq->opt is set to NULL, before calling inet_csk_route_child_sock(). Thus, inside inet_csk_route_child_sock() opt is always NULL and the SRR-options are not respected anymore. Packets sent by the server won't have the correct destination-IP. This patch fixes it by accessing newinet->inet_opt instead of ireq->opt inside inet_csk_route_child_sock(). Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be> --- net/ipv4/inet_connection_sock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)