Patchwork ipv4: Use netinet->inet_opt in inet_csk_route_child_sock()

login
register
mail settings
Submitter Christoph Paasch
Date Aug. 17, 2012, 9:35 p.m.
Message ID <1345239312-19266-1-git-send-email-christoph.paasch@uclouvain.be>
Download mbox | patch
Permalink /patch/178397/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Christoph Paasch - Aug. 17, 2012, 9:35 p.m.
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(-)
David Miller - Aug. 20, 2012, 9:51 a.m.
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
Christoph Paasch - Aug. 20, 2012, 12:11 p.m.
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

Patch

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;