Hi Staffan,
Thank you for this discovery!
It looks good, but I have a couple of comments.
It seems, there is one more problem in this code:
68 /* check for NULL path */
69 if (p == pathname) {
70 continue;<== Endless loop if we hit this line
71 }
Do we need to do 'pathname++' before continuing at the line #70?
It is going to be endless loop in cases there is a PATH_SEPARATOR
at the beginning of paths or two PATH_SEPARATOR's in a row.
These would be incorrect path lists but the code above is targeting
exactly such cases.
Also, the argument name "pathname" in the original code is confusing.
Should we rename it to "pathnames"?
Thanks,
Serguei
On 3/4/13 10:56 AM, Staffan Larsen wrote:
I accidentally stepped on this bug the other day. There is a problem in
linker_md.c : dll_build_name() where an internal pointer can be moved to point
outside the string. The code looks like:
57 static void dll_build_name(char* buffer, size_t buflen,
58 const char* pname, const char* fname) {
59 // Based on os_solaris.cpp
60
61 char *path_sep = PATH_SEPARATOR;
62 char *pathname = (char *)pname;
63 while (strlen(pathname) > 0) {
64 char *p = strchr(pathname, *path_sep);
65 if (p == NULL) {
66 p = pathname + strlen(pathname);
67 }
68 /* check for NULL path */
69 if (p == pathname) {
70 continue;
71 }
72 (void)snprintf(buffer, buflen, "%.*s/lib%s." LIB_SUFFIX, (p -
pathname),
73 pathname, fname);
74
75 if (access(buffer, F_OK) == 0) {
76 break;
77 }
78 pathname = p + 1;
79 *buffer = '\0';
80 }
If the supplied pname is a buffer with a simple path without any path
separators in it, p will be set to the terminating nul on line 66. Then on line
78 it will be moved outside the buffer. Fixing this also necessitates fixes to
the callers to check for an empty return string (in buffer).
The same code show up in both the solaris code and the windows code as well as
hprof.
bug: http://bugs.sun.com/view_bug.do?bug_id=8009397
webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/
Thanks,
/Staffan
