From owner-freebsd-hackers Mon Nov 11 9:23: 2 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 D272F37B401 for ; Mon, 11 Nov 2002 09:22:59 -0800 (PST) Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6DBBE43E75 for ; Mon, 11 Nov 2002 09:22:59 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: from apollo.backplane.com (localhost [127.0.0.1]) by apollo.backplane.com (8.12.5/8.12.5) with ESMTP id gABHMxFC033710; Mon, 11 Nov 2002 09:22:59 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.12.5/8.12.5/Submit) id gABHMw3c033707; Mon, 11 Nov 2002 09:22:58 -0800 (PST) (envelope-from dillon) Date: Mon, 11 Nov 2002 09:22:58 -0800 (PST) From: Matthew Dillon Message-Id: <200211111722.gABHMw3c033707@apollo.backplane.com> To: Michael Sinz Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Socket so_linger setting References: <3DC27247.5040100@wgate.com> <200211012008.gA1K8rOa034485@apollo.backplane.com> <3DCFD150.8080509@wgate.com> 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 I was going to wait till 5.0 released first but I could do it now if you want. -Matt : :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