Date: Thu, 02 Nov 2006 18:19:43 +0800 From: LI Xin <delphij@delphij.net> To: Max Laier <max@love2party.net> Cc: freebsd-net@freebsd.org, ??? <antinvidia@gmail.com> Subject: Re: Reentrant problem with inet_ntoa in the kernel Message-ID: <4549C63F.20308@delphij.net> In-Reply-To: <200611021045.09774.max@love2party.net> References: <be0088ce0611020026y4fe07749pd5a984f8744769b@mail.gmail.com> <200611021045.09774.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig8AB112A7DB7C773DA6DB8CEF Content-Type: multipart/mixed; boundary="------------090404090801090809060608" This is a multi-part message in MIME format. --------------090404090801090809060608 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 shoul= d >> be discussed here first? >=20 > In general, correct IPs in logs and debugging messages are a good thing= =2E =20 > I'm not sure, however, it is a good thing to put 17 bytes of buffer spa= ce=20 > on every function stack that might want to print an IP address. I thin= k=20 > it's less intrusive and equally good to have a hand full of static=20 > buffers available which are given out in a round-robin fashion - as=20 > attempted in ip6_sprintf. Obviously the buffer rotation needs to be=20 > atomic, though. If a caller needs the result for more than logging - o= r=20 > cares strongly - it can still allocate a private buffer and use the _r = > version. A general replacement of all applications of inet_ntoa just=20 > 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, --=20 Xin LI <delphij@delphij.net> http://www.delphij.net/ FreeBSD - The Power to Serve! --------------090404090801090809060608 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0"; name="patch-inet_ntoa.c" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="patch-inet_ntoa.c" Index: inet_ntoa.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 @@ =20 #include <netinet/in.h> =20 +static int ip4round =3D 0; char * inet_ntoa(struct in_addr ina) { - static char buf[4*sizeof "123"]; + static char ip4buf[8][4*sizeof "123"]; + char *buf =3D NULL; unsigned char *ucp =3D (unsigned char *)&ina; =20 + ip4round =3D (ip4round + 1) & 7; + buf =3D ip4buf[ip4round]; + sprintf(buf, "%d.%d.%d.%d", ucp[0] & 0xff, ucp[1] & 0xff, --------------090404090801090809060608-- --------------enig8AB112A7DB7C773DA6DB8CEF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFFScY/OfuToMruuMARA4x/AJ0QsrnUrj7fcb1HUE7qUzLtvFJobQCdGnEA S19ueTlRZd+odJberp9VN7E= =w8Bb -----END PGP SIGNATURE----- --------------enig8AB112A7DB7C773DA6DB8CEF--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4549C63F.20308>