Reentrant problem with inet_ntoa in the kernel

[Brooks Davis]

On Sat, Nov 04, 2006 at 02:46:30AM +0000, MQ wrote:
> 2006/11/3, Brooks Davis <brooks at one-eyed-alien.net>:
> >
> >On Fri, Nov 03, 2006 at 09:46:47AM +0000, 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?
> >
> >The particular definition used is excedingly ugly.  At a minimum there
> >needs to be a define or a constant "16" for the lenght rather than the
> >4*sizeof("123") nonsense.
> >
> >-- Brooks
> >
> >
> >
> Oh, I see. This kind of definition is really annoying, and hard to keep all
> the
> occurrences consistent. Maybe a better way is use a macro to make that
> clear?
> 
> #define IPV4_ADDRSZ (4 * sizeof "123")
> char buf[IPV4_ADDRSZ];
> 
> This "ugly" definition comes from inet_ntoa() in /sys/libkern/inet_ntoa.c,
> I just copied the style without too much consideration, it's my fault.

I'd just use 16.  The inet_ntoa function is frankly inane.  It attempts
to support chars that aren't 8 bits which would break so much stuff it
isn't funny.

-- Brooks
-------------- 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/20061105/9c850513/attachment.pgp