array overflow in local.c
Hi, gcc found a problem in our native/jni/javanet/local.c. I changed it to what I think makes sense but I am not sure whether this is still the intended behavior. Furthermore since overrunning the bounds of a stack allocated array may trash other stuff on the stack I wonder whether this fix also prevents the problem that the workaround above the modified code speaks of. Since I do not run Darwin-based OS I cannot test it myself. Regards Robert Index: classpath-0.96.1/native/jni/java-net/local.c === --- classpath-0.96.1.orig/native/jni/java-net/local.c 2008-06-27 11:21:31.0 +0200 +++ classpath-0.96.1/native/jni/java-net/local.c 2008-06-27 11:21:41.0 +0200 @@ -93,7 +93,7 @@ } strncpy (saddr.sun_path, addr, sizeof (saddr.sun_path)); - saddr.sun_path[sizeof (saddr.sun_path)] = '\0'; + saddr.sun_path[sizeof (saddr.sun_path) - 1] = '\0'; saddr.sun_family = AF_LOCAL; return bind (fd, (struct sockaddr *) saddr, SUN_LEN (saddr)); signature.asc Description: OpenPGP digital signature
Re: array overflow in local.c
Robert Schuster wrote:
gcc found a problem in our native/jni/javanet/local.c.
I changed it to what I think makes sense but I am not sure whether this
is still the intended behavior.
Furthermore since overrunning the bounds of a stack allocated array may
trash other stuff on the stack I wonder whether this fix also prevents
the problem that the workaround above the modified code speaks of. Since
I do not run Darwin-based OS I cannot test it myself.
That may well be right.
IMO it should be more like
Index: local.c
===
RCS file: /cvsroot/classpath/classpath/native/jni/java-net/local.c,v
retrieving revision 1.4
diff -u -r1.4 local.c
--- local.c 17 Apr 2007 21:46:27 - 1.4
+++ local.c 27 Jun 2008 10:21:12 -
@@ -86,14 +86,13 @@
if (gcc_sucks)
fprintf (stderr, bind %p\n, addr);
- if (strlen (addr) sizeof (saddr.sun_path))
+ if (strlen (addr) = sizeof (saddr.sun_path))
{
errno = ENAMETOOLONG;
return -1;
}
- strncpy (saddr.sun_path, addr, sizeof (saddr.sun_path));
- saddr.sun_path[sizeof (saddr.sun_path)] = '\0';
+ strcpy (saddr.sun_path, addr);
saddr.sun_family = AF_LOCAL;
return bind (fd, (struct sockaddr *) saddr, SUN_LEN (saddr));
Re: array overflow in local.c
Hi.
Andrew Haley schrieb:
Furthermore since overrunning the bounds of a stack allocated array may
trash other stuff on the stack I wonder whether this fix also prevents
the problem that the workaround above the modified code speaks of. Since
I do not run Darwin-based OS I cannot test it myself.
That may well be right.
IMO it should be more like
OK.
Casey would you mind testing the attached patch on your Darwin platform?
Regards
Robert
Index: native/jni/java-net/local.c
===
RCS file: /sources/classpath/classpath/native/jni/java-net/local.c,v
retrieving revision 1.4
diff -u -r1.4 local.c
--- native/jni/java-net/local.c 17 Apr 2007 21:46:27 - 1.4
+++ native/jni/java-net/local.c 27 Jun 2008 13:14:40 -
@@ -73,27 +73,18 @@
return socket (PF_UNIX, stream ? SOCK_STREAM : SOCK_DGRAM, 0);
}
-static int gcc_sucks = 0;
-
int
local_bind (int fd, const char *addr)
{
struct sockaddr_un saddr;
- /* For some reason, GCC 4.0.1 on Darwin/x86 MODIFIES the `addr'
- pointer in the CALLER's STACK FRAME after calling this function,
- but if we add this statement below, it doesn't! */
- if (gcc_sucks)
-fprintf (stderr, bind %p\n, addr);
-
- if (strlen (addr) sizeof (saddr.sun_path))
+ if (strlen (addr) = sizeof (saddr.sun_path))
{
errno = ENAMETOOLONG;
return -1;
}
- strncpy (saddr.sun_path, addr, sizeof (saddr.sun_path));
- saddr.sun_path[sizeof (saddr.sun_path)] = '\0';
+ strcpy (saddr.sun_path, addr);
saddr.sun_family = AF_LOCAL;
return bind (fd, (struct sockaddr *) saddr, SUN_LEN (saddr));
signature.asc
Description: OpenPGP digital signature
Re: array overflow in local.c
On Jun 27, 2008, at 6:15 AM, Robert Schuster wrote: Hi. Andrew Haley schrieb: Furthermore since overrunning the bounds of a stack allocated array may trash other stuff on the stack I wonder whether this fix also prevents the problem that the workaround above the modified code speaks of. Since I do not run Darwin-based OS I cannot test it myself. That may well be right. IMO it should be more like OK. Casey would you mind testing the attached patch on your Darwin platform? I'll give it a try when I get a chance, but this patch looks fine as is. (And I'm embarrassed; `addr' was being set to NULL, if I remember correctly)
