Reentrant problem with inet_ntoa in the kernel

MQ antinvidia at gmail.com
Sat Nov 4 03:01:03 UTC 2006


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 at 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 at 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 at freebsd.org
> \ /  Max Laier                          | ICQ #67774661
> X   http://pf4freebsd.love2party.net/  | mlaier at 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.


More information about the freebsd-net mailing list