Reentrant problem with inet_ntoa in the kernel

[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 <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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-net/attachments/20061103/0d61322a/attachment.pgp