From owner-freebsd-net@FreeBSD.ORG Fri Nov 3 09:31:18 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 4384516A47B for ; Fri, 3 Nov 2006 09:31:18 +0000 (UTC) (envelope-from antinvidia@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.186]) by mx1.FreeBSD.org (Postfix) with ESMTP id D803C43D77 for ; Fri, 3 Nov 2006 09:31:15 +0000 (GMT) (envelope-from antinvidia@gmail.com) Received: by nf-out-0910.google.com with SMTP id i2so1361920nfe for ; Fri, 03 Nov 2006 01:31:14 -0800 (PST) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:references; b=kO9/W0LIr54i9B4dueTcmdPom5lozyrtbAxarOBIduxoew6nD/GtGVlNm9HPh6vR0oagCrN6cyn01m4v7dasn5yabPJbi6U96aDdotRnlqdTdt03x+LWtHq9Z6AdkxifbQbsKsCOLX3s1P1oYJxDXelVM8MHXb9s5QgIbiXSK+U= Received: by 10.49.92.18 with SMTP id u18mr977509nfl.1162546273785; Fri, 03 Nov 2006 01:31:13 -0800 (PST) Received: by 10.49.37.15 with HTTP; Fri, 3 Nov 2006 01:31:13 -0800 (PST) Message-ID: Date: Fri, 3 Nov 2006 09:31:13 +0000 From: MQ To: "Max Laier" In-Reply-To: <200611021045.09774.max@love2party.net> MIME-Version: 1.0 References: <200611021045.09774.max@love2party.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: 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: Fri, 03 Nov 2006 09:31:18 -0000 2006/11/2, Max Laier : > > On Thursday 02 November 2006 09:26, . 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-n > >et > > > > I've already sent to PR(kern/104738), but got no reply, maybe it should > > be discussed here first? > > In general, correct IPs in logs and debugging messages are a good thing. > I'm not sure, however, it is a good thing to put 17 bytes of buffer space > on every function stack that might want to print an IP address. I think > it's less intrusive and equally good to have a hand full of static > buffers available which are given out in a round-robin fashion - as > attempted in ip6_sprintf. Obviously the buffer rotation needs to be > atomic, though. If a caller needs the result for more than logging - or > cares strongly - it can still allocate a private buffer and use the _r > version. A general replacement of all applications of inet_ntoa just > seems bloat. > > -- > /"\ Best regards, | mlaier@freebsd.org > \ / Max Laier | ICQ #67774661 > X http://pf4freebsd.love2party.net/ | mlaier@EFnet > / \ ASCII Ribbon Campaign | Against HTML Mail and News Yes, not all the occurrence of inet_ntoa should be replaced. I've replaced several inet_ntoa in /sys/boot/ just to be consistent with other files. MAYBE they will never be corrupted by *simultaneous*ly calling inet_ntoa. But the calls in the network stack can really break the results. If that happens, the users will be confused by the strange outputs and/or logs. So I think adding 16 bytes of memory consumption on the stack is worthy.