control: forwarded -1 https://sourceware.org/bugzilla/show_bug.cgi?id=18093
On 2015-02-25 13:49, Lennart Sorensen wrote: > I looked at ways the aux-cache could cause a segfault, and given the > file is mmap'd and has data offsets in it that are used as pointers > without being checked it is not hard to see how a corrupt file could > cause a segfault. The following patch makes the segfaults I was able > to think of and create go away. Thanks for your work, it's nice we have been able to understand the real issue. > I also have included an example corrupted aux-cache file > (aux-cache-corrupt-soname-offset) which has a bad offset that causes > a segfault. Unfortunately they doesn't seem to work here. That said I have been able to reproduce the problem by truncating/changing the aux cache manually. > There is another problem which I haven't solved but which is not a > segfault. If you somehow truncate the aux-cache file by a bit and run > the previous ldconfig without my patch, then you end up with a corrupted > aux-cache where some entries do not have soname's even though they should, > and that causes you to get messages like: > > /sbin/ldconfig.real: /lib/i386-linux-gnu/ is not a symbolic link > > /sbin/ldconfig.real: /usr/lib/i386-linux-gnu/ is not a symbolic link > > /sbin/ldconfig.real: /lib64/ is not a symbolic link > > /sbin/ldconfig.real: /usr/lib64/ is not a symbolic link > > /sbin/ldconfig.real: /libx32/ is not a symbolic link > > /sbin/ldconfig.real: /usr/libx32/ is not a symbolic link > > /sbin/ldconfig.real: /usr/lib/ is not a symbolic link > > /sbin/ldconfig.real: /usr/lib/i386-linux-gnu/i586/ is not a symbolic link > > /sbin/ldconfig.real: /usr/lib/i386-linux-gnu/i686/cmov/ is not a symbolic link > > Using ldconfig -i (and hence ignoring the corrupted aux-cache) makes > that problem go away. To solve it would of course mean you have to > not trust the cache which rather defeats the point of having the cache, > so I don't know if that is worth trying to solve. It does not cause a > segfault however. > > Using ldconfig -p to show the cache at that point has entries that are > clearly wrong such as: > > ... > day (libc6, OS ABI: Linux 2.6.32) => /lib/i386-linux-gnu/day > __kernel_sigreturn (libc6,x32, OS ABI: Linux 3.4.0) => > /libx32/__kernel_sigreturn > X_2.6 (libc6) => /usr/lib/i386-linux-gnu/X_2.6 > LF (libc6) => /usr/lib/i386-linux-gnu/LF > (libc6) => /usr/lib/ > (libc6, OS ABI: Linux 2.6.32) => /lib/i386-linux-gnu/ > (libc6, OS ABI: Linux 2.6.32) => /usr/lib/i386-linux-gnu/ > (libc6) => /lib/i386-linux-gnu/ > (libc6) => /usr/lib/i386-linux-gnu/ > (libc6) => /usr/lib/i386-linux-gnu/ > (libc6) => /usr/lib/i386-linux-gnu/ > (libc6) => /usr/lib/i386-linux-gnu/ > (libc6, OS ABI: Linux 2.6.32) => /lib/i386-linux-gnu/ > (libc6, OS ABI: Linux 2.6.32) => /usr/lib/i386-linux-gnu/ > (libc6,x32, OS ABI: Linux 3.4.0) => /libx32/ > (libc6,x32, OS ABI: Linux 3.4.0) => /usr/libx32/ > (libc6,x86-64, OS ABI: Linux 2.6.32) => /lib64/ > (libc6,x86-64, OS ABI: Linux 2.6.32) => /usr/lib64/ > (libc6, hwcap: 0x0008000000008000) => > /usr/lib/i386-linux-gnu/i686/cmov/ > (libc6, hwcap: 0x0004000000000000) => /usr/lib/i386-linux-gnu/i586/ > (libc6) => /lib/i386-linux-gnu/ > (libc6) => /usr/lib/i386-linux-gnu/ > (libc6) => /usr/lib/ > � (libc6) => /lib/i386-linux-gnu/� > > The file aux-cache-missing-sonames shows this corrupted state. > > I hope the patch at least helps solve the worst part of the problem. I think the idea behind the patch is correct, but it is not fully correct (or at least it only fixes corner cases). > diff -ur --exclude debian --exclude build-tree glibc-2.19.ori/elf/cache.c > glibc-2.19/elf/cache.c > --- glibc-2.19.ori/elf/cache.c 2015-02-25 16:24:59.000000000 +0000 > +++ glibc-2.19/elf/cache.c 2015-02-25 17:42:18.000000000 +0000 > @@ -699,7 +699,8 @@ > if (aux_cache == MAP_FAILED > || aux_cache_size < sizeof (struct aux_cache_file) > || memcmp (aux_cache->magic, AUX_CACHEMAGIC, sizeof AUX_CACHEMAGIC - 1) > - || aux_cache->nlibs >= aux_cache_size) > + || sizeof(struct aux_cache_file) + (aux_cache->nlibs - 1) * > sizeof(struct aux_cache_file_entry) >= aux_cache_size Why using (aux_cache->nlibs - 1) and not directly aux_cache->nlibs here? > + || aux_cache->nlibs * sizeof(struct aux_cache_file_entry) + > aux_cache->libs[aux_cache->nlibs - 1].soname >= aux_cache_size) The best to catch all cases here is to compute the theoretical size of the file using the headers and comparing it to the real one. > { > close (fd); > init_aux_cache (); > @@ -712,12 +713,14 @@ > const char *aux_cache_data > = (const char *) &aux_cache->libs[aux_cache->nlibs]; > for (unsigned int i = 0; i < aux_cache->nlibs; ++i) > - insert_to_aux_cache (&aux_cache->libs[i].id, > - aux_cache->libs[i].flags, > - aux_cache->libs[i].osversion, > - aux_cache->libs[i].soname == 0 > - ? NULL : aux_cache_data + aux_cache->libs[i].soname, > - 0); > + /* Only use entries with sane offsets */ > + if (aux_cache->libs[i].soname < aux_cache_size) The check is incorrect here, the address in aux_cache->libs[i].soname is relative to aux_cache_data, so it probably catches very few cases. > + insert_to_aux_cache (&aux_cache->libs[i].id, > + aux_cache->libs[i].flags, > + aux_cache->libs[i].osversion, > + aux_cache->libs[i].soname == 0 > + ? NULL : aux_cache_data + aux_cache->libs[i].soname, > + 0); > > munmap (aux_cache, aux_cache_size); > close (fd); Please find a new patch below. I have submitted it upstream as bz 18093. I am planning to wait for upstream answer or comment for other people a few days. I'll then prepare an upload fixing this bug. diff --git a/ChangeLog b/ChangeLog index 4a5cd16..5086267 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2015-03-08 Aurelien Jarno <aurel...@aurel32.net> + + [BZ #18093] + * elf/cache.c (load_aux_cache): Regenerate the cache if it has the + wrong size. Ignore entries pointing outside of the mmaped memory. + 2015-03-08 Paul Pluzhnikov <ppluzhni...@google.com> [BZ #16734] diff --git a/elf/cache.c b/elf/cache.c index 1732268..9131e08 100644 --- a/elf/cache.c +++ b/elf/cache.c @@ -698,7 +698,9 @@ load_aux_cache (const char *aux_cache_name) if (aux_cache == MAP_FAILED || aux_cache_size < sizeof (struct aux_cache_file) || memcmp (aux_cache->magic, AUX_CACHEMAGIC, sizeof AUX_CACHEMAGIC - 1) - || aux_cache->nlibs >= aux_cache_size) + || aux_cache_size != (sizeof(struct aux_cache_file) + + aux_cache->nlibs * sizeof(struct aux_cache_file_entry) + + aux_cache->len_strings)) { close (fd); init_aux_cache (); @@ -711,12 +713,13 @@ load_aux_cache (const char *aux_cache_name) const char *aux_cache_data = (const char *) &aux_cache->libs[aux_cache->nlibs]; for (unsigned int i = 0; i < aux_cache->nlibs; ++i) - insert_to_aux_cache (&aux_cache->libs[i].id, - aux_cache->libs[i].flags, - aux_cache->libs[i].osversion, - aux_cache->libs[i].soname == 0 - ? NULL : aux_cache_data + aux_cache->libs[i].soname, - 0); + if (aux_cache->libs[i].soname < aux_cache->len_strings) + insert_to_aux_cache (&aux_cache->libs[i].id, + aux_cache->libs[i].flags, + aux_cache->libs[i].osversion, + aux_cache->libs[i].soname == 0 + ? NULL : aux_cache_data + aux_cache->libs[i].soname, + 0); munmap (aux_cache, aux_cache_size); close (fd); -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org