From owner-freebsd-hackers Fri Nov 1 12: 8:59 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 98FA637B401 for ; Fri, 1 Nov 2002 12:08:56 -0800 (PST) Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3E54743E8A for ; Fri, 1 Nov 2002 12:08:56 -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 gA1K8rFC034486; Fri, 1 Nov 2002 12:08:54 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.12.5/8.12.5/Submit) id gA1K8rOa034485; Fri, 1 Nov 2002 12:08:53 -0800 (PST) (envelope-from dillon) Date: Fri, 1 Nov 2002 12:08:53 -0800 (PST) From: Matthew Dillon Message-Id: <200211012008.gA1K8rOa034485@apollo.backplane.com> To: Michael Sinz Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Socket so_linger setting References: <3DC27247.5040100@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 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? -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