Bug#759530: Patch to fix segfault in ldconfig
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: 0x00088000) =
/usr/lib/i386-linux-gnu/i686/cmov/
(libc6, hwcap: 0x0004) = /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.c2015-02-25 16:24:59.0 +
+++ glibc-2.19/elf/cache.c2015-02-25 17:42:18.0 +
@@ -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,
Processed: Re: Bug#759530: Patch to fix segfault in ldconfig
Processing control commands: forwarded -1 https://sourceware.org/bugzilla/show_bug.cgi?id=18093 Bug #759530 [libc-bin] libc-bin: ldconfig breaks a system Set Bug forwarded-to-address to 'https://sourceware.org/bugzilla/show_bug.cgi?id=18093'. -- 759530: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=759530 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#759530: Patch to fix segfault in ldconfig
On Sun, Mar 08, 2015 at 09:48:50PM +0100, Aurelien Jarno wrote:
Thanks for your work, it's nice we have been able to understand the real
issue.
Well I thing the best summary of the problem is that the aux-cache
handling code is awful and doesn't check anything before using the
incoming data as a source for pointers. Corrupt aux-cache files was
not even remotely considered. Safest option right now would be to disable
the use of the aux-cache if you want to avoid segfaults in ldconfig.
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.
Well it segfaulted on my amd64 system when I tried that file. Of course
it may need to have the same set of libraries installed as I have.
I think the idea behind the patch is correct, but it is not fully
correct (or at least it only fixes corner cases).
I don't doubt I missed some of the problem cases. Seems the best answer
would be to redesign it with a checksum and a size indicator or something
else that would catch corruption in general.
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.0 +
+++ glibc-2.19/elf/cache.c 2015-02-25 17:42:18.0 +
@@ -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?
Hmm, I seem to have misread the way the structure is defined, and thought
it always had one aux_cache_file_entry in the aux_cache_file struct,
but no it has 0 of them.
+ || 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.
Well the filenames seem to be just stored at the end of the other data
with pointers to it (which is where most of the chances for segfaults
occur). The format doesn't seem to actually have anything to cover the
size of that pile of strings. There is really no way to know what the
filesize should be given the end is just a heap of c strings. Did I
miss something? You seem to be using anpother header thing that mentions
the string lengths. Maybe I missed that existing. That would be useful.
{
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.
Well it catches any where the offset is corrupt by a larger value
(doesn't have to be that large either). But yes I did get that check
slightly wrong. I think I changed it a few times while working out what
the code was trying to do with the data structure.
+ 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
Bug#759530: Patch to fix segfault in ldconfig
On Sun, Mar 01, 2015 at 10:22:05PM +0100, Niels Thykier wrote: Excellent, thanks. I am taking the liberty of adding the patch tag for this one. If nothing else, I would greatly appreciate having ldconfig not seg. fault. :) That makes sense to me. Sounds like the aux-cache could do with a checksum or something to catch obvious corruptions. I very much agree, although that would of course be a format change and then you would have to throw away the old file on upgrades. Perfectly reasonable and probably the best way to deal with all the corner cases that I noticed for having it do the wrong thing when corrupted. I wonder what upstream thinks of it. Or ldconfig needs some method to (fairly) reliably detect corruptions. If it spits out errors about directories not being links, then something notices a problem. Perhaps this could be extended to discarding the cache (even if just a if (!ignore_cache) execve(argv, -i, null);). I thought that could work too. It sounded rather ugly to do that though. I rather like the checksum idea instead since it sounds more reliable and simpler. -- Len Sorensen -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#759530: Patch to fix segfault in ldconfig
Control: tags -1 patch upstream On Wed, 25 Feb 2015 13:49:00 -0500 Lennart Sorensen lsore...@csclub.uwaterloo.ca 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. I also have included an example corrupted aux-cache file (aux-cache-corrupt-soname-offset) which has a bad offset that causes a segfault. Excellent, thanks. I am taking the liberty of adding the patch tag for this one. If nothing else, I would greatly appreciate having ldconfig not seg. fault. :) 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: Sounds like the aux-cache could do with a checksum or something to catch obvious corruptions. [...] 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: [...] Or ldconfig needs some method to (fairly) reliably detect corruptions. If it spits out errors about directories not being links, then something notices a problem. Perhaps this could be extended to discarding the cache (even if just a if (!ignore_cache) execve(argv, -i, null);). Thanks, ~Niels -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#759530: Patch to fix segfault in ldconfig
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.
I also have included an example corrupted aux-cache file
(aux-cache-corrupt-soname-offset) which has a bad offset that causes
a segfault.
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: 0x00088000) =
/usr/lib/i386-linux-gnu/i686/cmov/
(libc6, hwcap: 0x0004) = /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.
--
Len Sorensen
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.0 +
+++ glibc-2.19/elf/cache.c 2015-02-25 17:42:18.0 +
@@ -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
+ || aux_cache-nlibs * sizeof(struct aux_cache_file_entry) + aux_cache-libs[aux_cache-nlibs - 1].soname = aux_cache_size)
{
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)
+ 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);
aux-cache-corrupt-soname-offset
Description: Binary data
aux-cache-missing-sonames
Description: Binary data
