From owner-freebsd-hackers Mon Nov 11 7:49: 1 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D7B3537B404 for ; Mon, 11 Nov 2002 07:48:58 -0800 (PST) Received: from MX1.wgate.com (mx1.wgate.com [66.150.46.4]) by mx1.FreeBSD.org (Postfix) with SMTP id 8A51B43E75 for ; Mon, 11 Nov 2002 07:48:57 -0800 (PST) (envelope-from msinz@wgate.com) Received: FROM mail.tvol.net BY MX1.wgate.com ; Mon Nov 11 10:42:51 2002 -0500 Received: from sinz.eng.tvol.net ([10.32.2.99]) by mail.tvol.net with SMTP (Microsoft Exchange Internet Mail Service Version 5.5.2653.13) id 41A6QWPY; Mon, 11 Nov 2002 10:48:45 -0500 Received: from wgate.com (sinz.eng.tvol.net [127.0.0.1]) by sinz.eng.tvol.net (8.12.5/8.12.5) with ESMTP id gABFmWPh001941; Mon, 11 Nov 2002 10:48:32 -0500 Message-ID: <3DCFD150.8080509@wgate.com> Date: Mon, 11 Nov 2002 10:48:32 -0500 From: Michael Sinz User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20021003 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Matthew Dillon Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Socket so_linger setting References: <3DC27247.5040100@wgate.com> <200211012008.gA1K8rOa034485@apollo.backplane.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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 > > > :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