Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[Nagendra Tomar]

This is the first part of the split up patch submitted in http://www.spinics.net/lists/netdev/msg142617.html

This patch fixes the sk_wait_event() condition in the sk_stream_wait_connect() function. With this change, we correctly check for the TCPF_ESTABLISHED and TCPF_CLOSE_WAIT states and avoid potentially returning success when there is an error on the socket.

Signed-off-by: Nagendra Singh Tomar <tomer_iisc@yahoo.com>
---
--- linux-2.6.35.7/net/core/stream.c.orig	2010-03-24 09:30:00.000000000 +0530
+++ linux-2.6.35.7/net/core/stream.c	2010-03-24 09:30:17.000000000 +0530
@@ -73,9 +73,8 @@ int sk_stream_wait_connect(struct sock *
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		sk->sk_write_pending++;
 		done = sk_wait_event(sk, timeo_p,
-				     !sk->sk_err &&
-				     !((1 << sk->sk_state) &
-				       ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
+				     ((1 << sk->sk_state) &
+				       (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
 		finish_wait(sk_sleep(sk), &wait);
 		sk->sk_write_pending--;
 	} while (!done);

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[David Miller]

Your patch is still corrupted, your email client is splitting up
long lines.

Please, save us a lot of time by test emailing the patch to yourself,
and then trying to apply the patch as you receive it.  Do this until
you're fixed all of the formatting problems and then you can send it
here.

Do not resend the patch by simply replying again to this thread,
send a fresh posting so that "Re: " doesn't show up in the subject
and this way I can apply it directly without any editing.

Thank you.

[PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[Nagendra Tomar]

This patch fixes the sk_wait_event() condition in the sk_stream_wait_connect() 
function. With this change, we correctly check for the TCPF_ESTABLISHED and 
TCPF_CLOSE_WAIT states and avoid potentially returning success when there
might be an error on the socket.

Signed-off-by: Nagendra Singh Tomar <tomer_iisc@xxxxxxxxx>
---
--- linux-2.6.35.7/net/core/stream.c.orig	2010-03-24 09:30:00.000000000 +0530
+++ linux-2.6.35.7/net/core/stream.c	2010-03-24 09:30:17.000000000 +0530
@@ -73,9 +73,8 @@ int sk_stream_wait_connect(struct sock *
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		sk->sk_write_pending++;
 		done = sk_wait_event(sk, timeo_p,
-				     !sk->sk_err &&
-				     !((1 << sk->sk_state) &
-				       ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
+				     ((1 << sk->sk_state) &
+				       (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
 		finish_wait(sk_sleep(sk), &wait);
 		sk->sk_write_pending--;
 	} while (!done);

---



      

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[Nagendra Tomar]

Dave,
     I had done the exercise of sending the patch to myself and applying it (copy-pasting just the patch). One thing that I see is the long line in the description. If you are referring to that, I've fixed it and submitted it again. If not this, I'm at loss.

Thanks,
Tomar

--- On Sun, 3/10/10, David Miller <davem@davemloft.net> wrote:

> From: David Miller <davem@davemloft.net>
> Subject: Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
> To: tomer_iisc@yahoo.com
> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
> Date: Sunday, 3 October, 2010, 1:56
> 
> Your patch is still corrupted, your email client is
> splitting up
> long lines.
> 
> Please, save us a lot of time by test emailing the patch to
> yourself,
> and then trying to apply the patch as you receive it. 
> Do this until
> you're fixed all of the formatting problems and then you
> can send it
> here.
> 
> Do not resend the patch by simply replying again to this
> thread,
> send a fresh posting so that "Re: " doesn't show up in the
> subject
> and this way I can apply it directly without any editing.
> 
> Thank you.
> 


      

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[David Miller]

From: Nagendra Tomar <tomer_iisc@yahoo.com>
Date: Sat, 2 Oct 2010 16:54:23 -0700 (PDT)

>      I had done the exercise of sending the patch to myself and
>      applying it (copy-pasting just the patch). One thing that I see
>      is the long line in the description. If you are referring to
>      that, I've fixed it and submitted it again. If not this, I'm at
>      loss.

This new submission looks good, thank you.

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[Nagendra Tomar]

Dave,
     Thinking more about it, we need to check for sk->sk_err, thus the 
existing code behaves fine. Just that we might incur an additional sleep
even while we know that the socket already has an error, but that should
be ok.
We only need the other patch. Pls ignore this, and sorry for the confusion.

Thanks,
Tomar

--- On Sun, 3/10/10, Nagendra Tomar <tomer_iisc@yahoo.com> wrote:

> From: Nagendra Tomar <tomer_iisc@yahoo.com>
> Subject: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()
> To: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org, davem@davemloft.net
> Date: Sunday, 3 October, 2010, 5:19
> This patch fixes the sk_wait_event()
> condition in the sk_stream_wait_connect() 
> function. With this change, we correctly check for the
> TCPF_ESTABLISHED and 
> TCPF_CLOSE_WAIT states and avoid potentially returning
> success when there
> might be an error on the socket.
> 
> Signed-off-by: Nagendra Singh Tomar
> <tomer_iisc@xxxxxxxxx>
> ---
> --- linux-2.6.35.7/net/core/stream.c.orig   
> 2010-03-24 09:30:00.000000000 +0530
> +++ linux-2.6.35.7/net/core/stream.c   
> 2010-03-24 09:30:17.000000000 +0530
> @@ -73,9 +73,8 @@ int sk_stream_wait_connect(struct sock *
>         
> prepare_to_wait(sk_sleep(sk), &wait,
> TASK_INTERRUPTIBLE);
>         
> sk->sk_write_pending++;
>          done =
> sk_wait_event(sk, timeo_p,
> -           
>          !sk->sk_err
> &&
> -           
>          !((1 <<
> sk->sk_state) &
> -           
>        
>    ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
> +           
>          ((1 <<
> sk->sk_state) &
> +           
>        
>    (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)));
>         
> finish_wait(sk_sleep(sk), &wait);
>         
> sk->sk_write_pending--;
>      } while (!done);
> 
> ---
> 
> 
> 
> 
> 



      

Re: [PATCH 1/2] net: Fix the condition passed to sk_wait_event()

[Eric Dumazet]

Le samedi 02 octobre 2010 à 17:06 -0700, David Miller a écrit :
> From: Nagendra Tomar <tomer_iisc@yahoo.com>
> Date: Sat, 2 Oct 2010 16:54:23 -0700 (PDT)
> 
> >      I had done the exercise of sending the patch to myself and
> >      applying it (copy-pasting just the patch). One thing that I see
> >      is the long line in the description. If you are referring to
> >      that, I've fixed it and submitted it again. If not this, I'm at
> >      loss.
> 
> This new submission looks good, thank you.

Yes, but the email address in the "Signed-odd-by: ...." is mangled :(