This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Sun, Dec 17, 2017 at 10:26:01PM +0100, Aurelien Jarno wrote: > On 2017-12-17 23:07, Dmitry V. Levin wrote: [...] > > > } > > > + else > > > + { > > > + to_free = cp = expand_dynamic_string_token (l, cp, 1); > > > + > > > + /* expand_dynamic_string_token can return NULL in case of memory > > > + allocation failure. */ > > > + if (cp == NULL) > > > + continue; > > > + > > > + /* Recompute the length after dynamic string token expansion > > > + and ignore empty paths. */ > > > + len = strlen (cp); > > > + if (len == 0) > > > + { > > > + free (to_free); > > > + continue; > > > + } > > > + } > > > > The alternative (that I was testing in my patch) to len == 0 check is to > > change expand_dynamic_string_token to return NULL if _dl_dst_substitute > > returned an empty string, e.g. > > > > - return _dl_dst_substitute (l, s, result, is_path); > > + char *retval = _dl_dst_substitute (l, s, result, is_path); > > + > > + /* If substitution of dynamic string tokens resulted to an empty string, > > + return NULL as in case of insufficient memory. */ > > + if (__glibc_unlikely (*retval == '\0')) > > + { > > + free (result); > > + return NULL; > > + } > > + > > + return retval; > > > > Both users of expand_dynamic_string_token, fillin_rpath and _dl_map_object, > > would be happy. > > I thought about that, but refrained as much as possible to change any > interface, as it might trigger more bugs, even if it looks ok. I agree in general, but in this particular case I've been playing with this code for some time and think the change would be safe. > The current bug actually show that a simple change might have big > consequences. Overall it seems the RPATH handling code became very > complex over the time, and would clearly benefit from a rewrite. I > would not be really surprised if other similar bugs are found. There are definitely more bugs in this code. For example, _dl_init_paths calls _dl_dst_substitute twice on $LD_LIBRARY_PATH: the first time _dl_dst_substitute is called directly, the second time the result is passed to fillin_rpath which calls expand_dynamic_string_token which in turn calls _dl_dst_substitute. As $LD_LIBRARY_PATH is filtered in case of __libc_enable_secure, this doesn't look security sensitive, it just does a wrong thing: $ mkdir -p /tmp/'$ORIGIN' && cd /tmp/'$ORIGIN' && echo 'int main(){}' |gcc -xc - && strace -qq -E LD_LIBRARY_PATH='$ORIGIN' -e /open ./a.out open("/tmp//tmp/$ORIGIN/tls/x86_64/libc.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) open("/tmp//tmp/$ORIGIN/tls/libc.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) open("/tmp//tmp/$ORIGIN/x86_64/libc.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) open("/tmp//tmp/$ORIGIN/libc.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3 open("/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 -- ldv
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |