Date: Mon, 11 Nov 2002 10:48:32 -0500 From: Michael Sinz <msinz@wgate.com> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Socket so_linger setting Message-ID: <3DCFD150.8080509@wgate.com> References: <3DC27247.5040100@wgate.com> <200211012008.gA1K8rOa034485@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Matthew Dillon wrote: > I think your patch is fine as is, Mike! Good find! Even though > so_linger cannot be negative, it is often convenient to use a signed > integer to store the value to avoid unexpected arithmatic results > when mixing with signed operations. My quick perusal does not show > any cases of this for so_linger, so we could make it unsigned, but I > don't see any pressing need to do so. The range check would need > to be in there in either case. > > Can I go ahead and commit it? What is the status with this? As far as I can tell, the fix is correct and needed for some Java/JCK issues (the issue can be worked around in the JVM but that is the incorrect place to deal with it) > -Matt > Matthew Dillon > <dillon@backplane.com> > > :During some parameter limit checking work, I ran into what I believe to > :be an error in FreeBSD. (Albeit unlikely to be hit) > : > :A setsockopt of the SO_LINGER field will cause strange results if > :the value is set above 32767. > : > :This is due to the fact that in struct socket, the so_linger field > :is a signed short and the parameter passed to setsockopt for linger > :is a signed long. > : > :What happens is that any value between 32768 and 65535 will cause > :so_linger to be negative. And then getsockopt will return a sign > :extended negative value in the signed long field for linger. > : > :The "trivial" fix is to do the following: > : > :------------------------------------------------------ > :--- uipc_socket.c Wed May 1 01:13:02 2002 > :+++ /tmp/uipc_socket.c Fri Nov 1 06:55:10 2002 > :@@ -1139,7 +1139,8 @@ > : if (error) > : goto bad; > : > :- so->so_linger = l.l_linger; > :+ /* Limit the value to what fits in so_linger */ > :+ so->so_linger = (l.l_linger > SHRT_MAX ? SHRT_MAX : l.linger); > : if (l.l_onoff) > : so->so_options |= SO_LINGER; > : else > :------------------------------------------------------ > : > :What this does is limit the value to no more than 32767 (SHRT_MAX) > :However, I believe the more correct answer is that so_linger should > :not be a signed value to begin with. > : > :The reasoning is that what does a negative so_linger mean? To > :close the socket before the user does ;^)? > : > :It is somewhat obvious that so_linger does not need to be a long. > : > :It is not possible to change the API to make the input a short. > : > :Limiting the value to 32767 is reasonable (and that is a *vary* > :long linger time) > : > :However, given that negative linger values really don't exist > :(logically) it would be reasonable to not that field be signed. > :That would naturally limit the values to being within a valid > :range and prevent some strange results, specifically when > :looking at the tsleep() call where the so_linger field is > :just blindly multiplied by the hz of the system. (around line > :312 of uipc_socket.c) A segative so_linger will get sign extended > :into a negative int (32-bit) (times hz) and then passed to tsleep > :which just checks for zero, passed on to timeout which then > :passes it to callout_reset. It turns out that callout_reset will > :take negative values and make them a single tick... (whew! lucky > :thing that was there :-) > : > :The question I have is: should put together a patch that changes > :so_linger (and xso_linger) to unsigned? (And make sure there are > :no bad side effects) or is the trivial fix above what is wanted? > : > :[ My personal feeling is that since so_linger has no valid negative > : value that the field should be unsigned. Not that it matters > : about improving the range as 32767 is over 9 hours. It is more > : a matter of "correctness" in the code/representation since the > : code assumes the value is not negative already. ] > : > :-- > :Michael Sinz -- Director, Systems Engineering -- Worldgate Communications > :A master's secrets are only as good as > : the master's ability to explain them to others. > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-hackers" in the body of the message To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3DCFD150.8080509>