All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipv4: Use netinet->inet_opt in inet_csk_route_child_sock()
@ 2012-08-17 21:35 Christoph Paasch
  2012-08-20  9:51 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Paasch @ 2012-08-17 21:35 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

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(-)

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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ipv4: Use netinet->inet_opt in inet_csk_route_child_sock()
  2012-08-17 21:35 [PATCH] ipv4: Use netinet->inet_opt in inet_csk_route_child_sock() Christoph Paasch
@ 2012-08-20  9:51 ` David Miller
  2012-08-20 12:11   ` Christoph Paasch
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2012-08-20  9:51 UTC (permalink / raw)
  To: christoph.paasch; +Cc: netdev

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ipv4: Use netinet->inet_opt in inet_csk_route_child_sock()
  2012-08-20  9:51 ` David Miller
@ 2012-08-20 12:11   ` Christoph Paasch
  0 siblings, 0 replies; 3+ messages in thread
From: Christoph Paasch @ 2012-08-20 12:11 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

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

-- 
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
Université Catholique de Louvain
--

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-08-20 12:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-17 21:35 [PATCH] ipv4: Use netinet->inet_opt in inet_csk_route_child_sock() Christoph Paasch
2012-08-20  9:51 ` David Miller
2012-08-20 12:11   ` Christoph Paasch

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.