From owner-freebsd-net@FreeBSD.ORG Sun Nov 5 01:09:44 2006 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A0D5816A417 for ; Sun, 5 Nov 2006 01:09:44 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from sccmmhc91.asp.att.net (sccmmhc91.asp.att.net [204.127.203.211]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4465E43D5E for ; Sun, 5 Nov 2006 01:09:42 +0000 (GMT) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net ([12.207.12.9]) by sccmmhc91.asp.att.net (sccmmhc91) with ESMTP id <20061105010940m9100el2n9e>; Sun, 5 Nov 2006 01:09:40 +0000 Received: from lor.one-eyed-alien.net (localhost [127.0.0.1]) by lor.one-eyed-alien.net (8.13.8/8.13.8) with ESMTP id kA519cUJ007537; Sat, 4 Nov 2006 19:09:38 -0600 (CST) (envelope-from brooks@lor.one-eyed-alien.net) Received: (from brooks@localhost) by lor.one-eyed-alien.net (8.13.8/8.13.8/Submit) id kA519aMm007536; Sat, 4 Nov 2006 19:09:36 -0600 (CST) (envelope-from brooks) Date: Sat, 4 Nov 2006 19:09:36 -0600 From: Brooks Davis To: MQ Message-ID: <20061105010936.GA6669@lor.one-eyed-alien.net> References: <20061102142543.GC70915@lor.one-eyed-alien.net> <200611031220.53791.max@love2party.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pWyiEgJYm5f9v55/" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.11 Cc: Max Laier , freebsd-net@freebsd.org Subject: Re: Reentrant problem with inet_ntoa in the kernel X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Nov 2006 01:09:44 -0000 --pWyiEgJYm5f9v55/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Nov 04, 2006 at 03:01:00AM +0000, MQ wrote: > I use google mail web interface to post messages, I can't connect to the > google mail POP server because someone disabled it on the firewall :( >=20 > I don't know if this post will be better? >=20 > 2006/11/3, Max Laier : > > > >Hello "MQ", > > > >your email client is seriously mis-configured, could you please look into > >this as it is a bit annoying. > > > >On Friday 03 November 2006 10:46, MQ wrote: > >> 2006/11/2, Brooks Davis : > >> > On Thu, Nov 02, 2006 at 08:26:27AM +0000, . wrote: > >> > > Hi, > >> > > > >> > > I am confused by the use of inet_ntoa function in the kernel. > >> > > > >> > > The function inet_ntoa in the /sys/libkern/inet_ntoa.c uses a > >> > > static > >> > > >> > array > >> > > >> > > static char buf[4 * sizeof "123"]; > >> > > to store the result. And it returns the address of the array to the > >> > > >> > caller. > >> > > >> > > I think this inet_ntoa is not reentrant, though there are several > >> > > >> > functions > >> > > >> > > calling it. If two functions call it simultaneously, the result > >> > > will be corrupted. Though I haven't really encountered this > >> > > situation, it may > >> > > >> > occur > >> > > >> > > someday, especially when using multi-processors. > >> > > > >> > > There is another reentrant version of inet_ntoa called inet_ntoa_r > >> > > in > >> > > >> > the > >> > > >> > > same file. It has been there for several years, but just used by > >> > > ipfw2 > >> > > >> > for > >> > > >> > > about four times in 7-CURRENT. In my patch, I replaced all the > >> > > calls to inet_ntoa with calls to inet_ntoa_r. > >> > > > >> > > By the way, some of the original calls is written in this style: > >> > > strcpy(buf, inet_ntoa(ip)) > >> > > The modified code is written in this style > >> > > inet_ntoa_r(ip, buf) > >> > > This change avoids a call to strcpy, and can save a little time. > >> > > > >> > > Here is the patch. > >> > > >> > http://people.freebsd.org/~delphij/misc/patch-itoa-by-nodummy-at-yeah > >> >-net > >> > > >> > > I've already sent to PR(kern/104738), but got no reply, maybe it > >> > > should > >> > > >> > be > >> > > >> > > discussed here first? > >> > > >> > I've got to agree with other posters that the stack variable > >> > allocations are ugly. What about extending log and printf to > >> > understand ip4v addresses? That's 90% of the uses and the others > >> > appears to have buffers already. > >> > > >> > -- Brooks > >> > > >> > >> Ugly? Why? Don't you use local variables in your sources? > > > >You have to understand, that stack space is a limited resource in the > >kernel. Some of the functions are leaf nodes in quite long call paths, > >which means adding those buffers can hit quite hard. > > > >I guess what Brooks and I are trying to say is, that this needs to be > >considered carefully. A simple search and replace won't do. > > > >BTW, I took the PR for now and will look into it. I don't think it's > >something we need to rush, as I haven't seen any reports or indication of > >real problems yet - fwiw we don't spew a lot of IP addresses to console > >or log in normal operation. > > > >> By the way, implementing a printf/log which understands ipv4 address is > >> tedious, perhaps. > > > >Actually, it's a ~15 line patch, I will see if I can get to it later > >today. If we reuse the number buffer we can even get away without > >increase in stack space usage. Whether or not this is a good idea is on > >another page, but %I and %lI (for IPv6) would be available and we already > >have %D and %b as special cases in the kernel. > > > I know for sure that the kernel stack is much smaller than that in > userland programms, but is the kernel stack too small to contain > another 32 bytes at most? I've no idea, and is there any way to trace > the usage of the kernel stack? KGDB? > > The reason why I don't like the idea that adding the facility into > printf is we already had the converter inet_ntoa, adding the facility > seems duplicated. But if implemented, and use a local buffer, this > should be a really good way to solve the reentrant problem. Given that except for a few cases that should have obviously used inet_ntoa_r, inet_ntoa is only used to feed printf and log, the right answer is clearly to implement it in printf and log and the nuke this fundamental broken API. -- Brooks --pWyiEgJYm5f9v55/ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (FreeBSD) iD8DBQFFTTnPXY6L6fI4GtQRAlzYAJ0UtouH4Y57Sx+JcVazUDAWHoRUfACeKvE1 cTJ1Jep4uRf4VCipMS2k3MY= =pjjJ -----END PGP SIGNATURE----- --pWyiEgJYm5f9v55/--