Date: Sat, 4 Nov 2006 03:01:00 +0000 From: MQ <antinvidia@gmail.com> To: "Max Laier" <max@love2party.net> Cc: freebsd-net@freebsd.org Subject: Re: Reentrant problem with inet_ntoa in the kernel Message-ID: <be0088ce0611031901g100a8e29k2e9728a8806cf385@mail.gmail.com> In-Reply-To: <200611031220.53791.max@love2party.net> References: <be0088ce0611020026y4fe07749pd5a984f8744769b@mail.gmail.com> <20061102142543.GC70915@lor.one-eyed-alien.net> <be0088ce0611030146u5e97e08cmbd36e94d772c8a94@mail.gmail.com> <200611031220.53791.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 :( I don't know if this post will be better? 2006/11/3, Max Laier <max@love2party.net>: > > 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 <brooks@one-eyed-alien.net>: > > > 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. > > -- > /"\ Best regards, | mlaier@freebsd.org > \ / Max Laier | ICQ #67774661 > X http://pf4freebsd.love2party.net/ | mlaier@EFnet > / \ ASCII Ribbon Campaign | Against HTML Mail and News > > > 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?be0088ce0611031901g100a8e29k2e9728a8806cf385>