Max Laier wrote: > 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.
Sounds like a workaround to me and in theory that is insufficient for a MPSAFE protection. Here is a patch which reduces the chance where we get a race. Cheers, -- Xin LI <[EMAIL PROTECTED]> http://www.delphij.net/ FreeBSD - The Power to Serve!
Index: inet_ntoa.c
===================================================================
RCS file: /home/ncvs/src/sys/libkern/inet_ntoa.c,v
retrieving revision 1.6
diff -u -r1.6 inet_ntoa.c
--- inet_ntoa.c 7 Jan 2005 00:24:32 -0000 1.6
+++ inet_ntoa.c 2 Nov 2006 10:00:45 -0000
@@ -35,12 +35,17 @@
#include <netinet/in.h>
+static int ip4round = 0;
char *
inet_ntoa(struct in_addr ina)
{
- static char buf[4*sizeof "123"];
+ static char ip4buf[8][4*sizeof "123"];
+ char *buf = NULL;
unsigned char *ucp = (unsigned char *)&ina;
+ ip4round = (ip4round + 1) & 7;
+ buf = ip4buf[ip4round];
+
sprintf(buf, "%d.%d.%d.%d",
ucp[0] & 0xff,
ucp[1] & 0xff,
signature.asc
Description: OpenPGP digital signature
