is sk->reuse truly a boolean?

is sk->reuse truly a boolean?

[Arnaldo Carvalho de Melo]

>From what I see in the code and from references in, for instance, Unix
Network Programming (W. Richard Stevens) it is, but then how can this work?

net/ipv4/tcp_ipv4.c, line 265

                if (sk->reuse > 1)
                        goto success;

In net/core/sock.c, setsockopt it just assigns 1 or 0, i.e. if userspace
passes > 1 it becomes 1, is this the intended behaviour? I think we have a 
bug in tcp_ipv4 or in core/sock.c 8)

Comments?

- Arnaldo

Re: is sk->reuse truly a boolean?

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20030520155744.GE801@conectiva.com.br> (at Tue, 20 May 2003 12:57:45 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:

> From what I see in the code and from references in, for instance, Unix
> Network Programming (W. Richard Stevens) it is, but then how can this work?
> 
> net/ipv4/tcp_ipv4.c, line 265
> 
>                 if (sk->reuse > 1)
>                         goto success;
> 
> In net/core/sock.c, setsockopt it just assigns 1 or 0, i.e. if userspace
> passes > 1 it becomes 1, is this the intended behaviour? I think we have a 
> bug in tcp_ipv4 or in core/sock.c 8)

Good point. However, SO_REUSEADDR works because we have tcp_bind_conflict().

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: is sk->reuse truly a boolean?

[Arnaldo Carvalho de Melo]

Em Wed, May 21, 2003 at 01:15:20AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ escreveu:
> In article <20030520155744.GE801@conectiva.com.br> (at Tue, 20 May 2003 12:57:45 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
> 
> > From what I see in the code and from references in, for instance, Unix
> > Network Programming (W. Richard Stevens) it is, but then how can this work?
> > 
> > net/ipv4/tcp_ipv4.c, line 265
> > 
> >                 if (sk->reuse > 1)
> >                         goto success;
> > 
> > In net/core/sock.c, setsockopt it just assigns 1 or 0, i.e. if userspace
> > passes > 1 it becomes 1, is this the intended behaviour? I think we have a 
> > bug in tcp_ipv4 or in core/sock.c 8)
> 
> Good point. However, SO_REUSEADDR works because we have tcp_bind_conflict().

mmmkay, so we have to fix it by changing the test to:

		    if (sk->reuse)
			   goto success;

Isn't it?

Another one I found in this audit:

sk->no_check, as per setsockopt is a boolean, but sunrpc code wants tree
values, does that need receiving this three values from userspace? if so we
have a bug in setsockopt, but without looking at sunrpc code I guess this is
only internal...

- Arnaldo

Re: is sk->reuse truly a boolean?

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20030520162906.GF801@conectiva.com.br> (at Tue, 20 May 2003 13:29:07 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:

> > > In net/core/sock.c, setsockopt it just assigns 1 or 0, i.e. if userspace
> > > passes > 1 it becomes 1, is this the intended behaviour? I think we have a 
> > > bug in tcp_ipv4 or in core/sock.c 8)
> > 
> > Good point. However, SO_REUSEADDR works because we have tcp_bind_conflict().
> 
> mmmkay, so we have to fix it by changing the test to:
> 
> 		    if (sk->reuse)
> 			   goto success;
> 
> Isn't it?

I don't think so.  Above modification will break current 
reasonable bind(2) behavior.

Well, it would be dead code, which would be used for (still 
unsupported) SO_REUSEPORT.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: is sk->reuse truly a boolean?

[Arnaldo Carvalho de Melo]

Em Wed, May 21, 2003 at 01:53:17AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ escreveu:
> In article <20030520162906.GF801@conectiva.com.br> (at Tue, 20 May 2003 13:29:07 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
> 
> > > > In net/core/sock.c, setsockopt it just assigns 1 or 0, i.e. if userspace
> > > > passes > 1 it becomes 1, is this the intended behaviour? I think we have a 
> > > > bug in tcp_ipv4 or in core/sock.c 8)
> > > 
> > > Good point. However, SO_REUSEADDR works because we have tcp_bind_conflict().
> > 
> > mmmkay, so we have to fix it by changing the test to:
> > 
> > 		    if (sk->reuse)
> > 			   goto success;
> > 
> > Isn't it?
> 
> I don't think so.  Above modification will break current 
> reasonable bind(2) behavior.
> 
> Well, it would be dead code, which would be used for (still 
> unsupported) SO_REUSEPORT.

So just delete the test?

- Arnaldo

Re: is sk->reuse truly a boolean?

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20030520165831.GG801@conectiva.com.br> (at Tue, 20 May 2003 13:58:31 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:

> > I don't think so.  Above modification will break current 
> > reasonable bind(2) behavior.
> > 
> > Well, it would be dead code, which would be used for (still 
> > unsupported) SO_REUSEPORT.
> 
> So just delete the test?

Hmm, (bacause I want to remember this path in the code,) how about #if 0 ?
David?

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: is sk->reuse truly a boolean?

[Arnaldo Carvalho de Melo]

Em Wed, May 21, 2003 at 02:02:42AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ escreveu:
> In article <20030520165831.GG801@conectiva.com.br> (at Tue, 20 May 2003 13:58:31 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
> 
> > > I don't think so.  Above modification will break current 
> > > reasonable bind(2) behavior.
> > > 
> > > Well, it would be dead code, which would be used for (still 
> > > unsupported) SO_REUSEPORT.
> > 
> > So just delete the test?
> 
> Hmm, (bacause I want to remember this path in the code,) how about #if 0 ?
> David?

Sounds good to me.

Re: is sk->reuse truly a boolean?

[David S. Miller]

   From: YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org>
   Date: Wed, 21 May 2003 02:02:42 +0900 (JST)

   In article <20030520165831.GG801@conectiva.com.br> (at Tue, 20 May 2003 13:58:31 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
   
   > So just delete the test?
   
   Hmm, (bacause I want to remember this path in the code,) how about #if 0 ?
   David?

You may #if 0 this as long as you:

1) Add comment.
2) Add identical code block to tcp_ipv6.c

Re: is sk->reuse truly a boolean?

[David S. Miller]

   From: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
   Date: Tue, 20 May 2003 13:29:07 -0300

   Another one I found in this audit:
   
   sk->no_check, as per setsockopt is a boolean, but sunrpc code wants tree
   values, does that need receiving this three values from userspace? if so we
   have a bug in setsockopt, but without looking at sunrpc code I guess this is
   only internal...

This UDP_CSUM_NORCV value made sense when we did UDP checksumming
at packet receive time, but now that we delay checksumming until
user recvmsg() call runs it is quite useless.

net/sunrpc/xprt.c:udp_data_ready() wants to see packets which have
not been checksummed yet, so it may do this and copy at the same
time.  This is the only behavior made by UDP, it never checks
sk->no_check value.

So UDP_CSUM_NORCV and the sk->no_check setting in net/sunrpc/xprt.c
can be both deleted.  It is true for 2.4.x too, and you probably want
to run this patch by the NFS guys first so they do not have a heart
attack when they notice this hit the tree :-)