Reentrant problem with inet_ntoa in the kernel

[MQ]

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