commit 86013ef5cea322b8f4b9c22f230c22cce369e947 Author: Carlos O'Donell Date: Mon Jan 21 22:50:12 2019 -0500 nptl: Fix pthread_rwlock_try*lock stalls (Bug 23844) For a full analysis of both the pthread_rwlock_tryrdlock() stall and the pthread_rwlock_trywrlock() stall see: https://sourceware.org/bugzilla/show_bug.cgi?id=23844#c14 In the pthread_rwlock_trydlock() function we fail to inspect for PTHREAD_RWLOCK_FUTEX_USED in __wrphase_futex and wake the waiting readers. In the pthread_rwlock_trywrlock() function we write 1 to __wrphase_futex and loose the setting of the PTHREAD_RWLOCK_FUTEX_USED bit, again failing to wake waiting readers during unlock. The fix in the case of pthread_rwlock_trydlock() is to check for PTHREAD_RWLOCK_FUTEX_USED and wake the readers. The fix in the case of pthread_rwlock_trywrlock() is to only write 1 to __wrphase_futex if we installed the write phase, since all other readers would be spinning waiting for this step. We add two new tests, one exercises the stall for pthread_rwlock_trywrlock() which is easy to exercise, and one exercises the stall for pthread_rwlock_trydlock() which is harder to exercise. The pthread_rwlock_trywrlock() test fails consistently without the fix, and passes after. The pthread_rwlock_tryrdlock() test fails roughly 5-10% of the time without the fix, and passes all the time after. Signed-off-by: Carlos O'Donell Signed-off-by: Torvald Riegel Signed-off-by: Rik Prohaska Co-authored-by: Torvald Riegel Co-authored-by: Rik Prohaska (cherry picked from commit 5fc9ed4c4058bfbdf51ad6e7aac7d209b580e8c4) diff --git a/ChangeLog b/ChangeLog index 59dab18463..adb4e719a6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2019-01-31 Carlos O'Donell + Torvald Riegel + Rik Prohaska + + [BZ# 23844] + * nptl/Makefile (tests): Add tst-rwlock-tryrdlock-stall, and + tst-rwlock-trywrlock-stall. + * nptl/pthread_rwlock_tryrdlock.c (__pthread_rwlock_tryrdlock): + Wake waiters if PTHREAD_RWLOCK_FUTEX_USED is set. + * nptl/pthread_rwlock_trywrlock.c (__pthread_rwlock_trywrlock): + Set __wrphase_fute to 1 only if we started the write phase. + * nptl/tst-rwlock-tryrdlock-stall.c: New file. + * nptl/tst-rwlock-trywrlock-stall.c: New file. + * support/Makefile (libsupport-routines): Add xpthread_rwlock_destroy. + * support/xpthread_rwlock_destroy.c: New file. + * support/xthread.h: Declare xpthread_rwlock_destroy. + 2019-01-31 Siddhesh Poyarekar * version.h (RELEASE): Set to "stable". diff --git a/nptl/Makefile b/nptl/Makefile index 340282c6cb..0e316edfac 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -319,7 +319,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \ tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-cnd-timedwait tst-thrd-detach tst-mtx-basic tst-thrd-sleep \ tst-mtx-recursive tst-tss-basic tst-call-once tst-mtx-timedlock \ - tst-rwlock-pwn + tst-rwlock-pwn \ + tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall tests-internal := tst-rwlock19 tst-rwlock20 \ tst-sem11 tst-sem12 tst-sem13 \ diff --git a/nptl/pthread_rwlock_tryrdlock.c b/nptl/pthread_rwlock_tryrdlock.c index 368862ff07..2f94f17f36 100644 --- a/nptl/pthread_rwlock_tryrdlock.c +++ b/nptl/pthread_rwlock_tryrdlock.c @@ -94,15 +94,22 @@ __pthread_rwlock_tryrdlock (pthread_rwlock_t *rwlock) /* Same as in __pthread_rwlock_rdlock_full: We started the read phase, so we are also responsible for updating the write-phase futex. Relaxed MO is sufficient. - Note that there can be no other reader that we have to wake - because all other readers will see the read phase started by us - (or they will try to start it themselves); if a writer started - the read phase, we cannot have started it. Furthermore, we - cannot discard a PTHREAD_RWLOCK_FUTEX_USED flag because we will - overwrite the value set by the most recent writer (or the readers - before it in case of explicit hand-over) and we know that there - are no waiting readers. */ - atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 0); + We have to do the same steps as a writer would when handing over the + read phase to use because other readers cannot distinguish between + us and the writer. + Note that __pthread_rwlock_tryrdlock callers will not have to be + woken up because they will either see the read phase started by us + or they will try to start it themselves; however, callers of + __pthread_rwlock_rdlock_full just increase the reader count and then + check what state the lock is in, so they cannot distinguish between + us and a writer that acquired and released the lock in the + meantime. */ + if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0) + & PTHREAD_RWLOCK_FUTEX_USED) != 0) + { + int private = __pthread_rwlock_get_private (rwlock); + futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private); + } } return 0; diff --git a/nptl/pthread_rwlock_trywrlock.c b/nptl/pthread_rwlock_trywrlock.c index fd37a71ce4..fae475cc70 100644 --- a/nptl/pthread_rwlock_trywrlock.c +++ b/nptl/pthread_rwlock_trywrlock.c @@ -46,8 +46,15 @@ __pthread_rwlock_trywrlock (pthread_rwlock_t *rwlock) &rwlock->__data.__readers, &r, r | PTHREAD_RWLOCK_WRPHASE | PTHREAD_RWLOCK_WRLOCKED)) { + /* We have become the primary writer and we cannot have shared + the PTHREAD_RWLOCK_FUTEX_USED flag with someone else, so we + can simply enable blocking (see full wrlock code). */ atomic_store_relaxed (&rwlock->__data.__writers_futex, 1); - atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1); + /* If we started a write phase, we need to enable readers to + wait. If we did not, we must not change it because other threads + may have set the PTHREAD_RWLOCK_FUTEX_USED in the meantime. */ + if ((r & PTHREAD_RWLOCK_WRPHASE) == 0) + atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1); atomic_store_relaxed (&rwlock->__data.__cur_writer, THREAD_GETMEM (THREAD_SELF, tid)); return 0; diff --git a/nptl/tst-rwlock-tryrdlock-stall.c b/nptl/tst-rwlock-tryrdlock-stall.c new file mode 100644 index 0000000000..5e476da2b8 --- /dev/null +++ b/nptl/tst-rwlock-tryrdlock-stall.c @@ -0,0 +1,355 @@ +/* Bug 23844: Test for pthread_rwlock_tryrdlock stalls. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* For a full analysis see comment: + https://sourceware.org/bugzilla/show_bug.cgi?id=23844#c14 + + Provided here for reference: + + --- Analysis of pthread_rwlock_tryrdlock() stall --- + A read lock begins to execute. + + In __pthread_rwlock_rdlock_full: + + We can attempt a read lock, but find that the lock is + in a write phase (PTHREAD_RWLOCK_WRPHASE, or WP-bit + is set), and the lock is held by a primary writer + (PTHREAD_RWLOCK_WRLOCKED is set). In this case we must + wait for explicit hand over from the writer to us or + one of the other waiters. The read lock threads are + about to execute: + + 341 r = (atomic_fetch_add_acquire (&rwlock->__data.__readers, + 342 (1 << PTHREAD_RWLOCK_READER_SHIFT)) + 343 + (1 << PTHREAD_RWLOCK_READER_SHIFT)); + + An unlock beings to execute. + + Then in __pthread_rwlock_wrunlock: + + 547 unsigned int r = atomic_load_relaxed (&rwlock->__data.__readers); + ... + 549 while (!atomic_compare_exchange_weak_release + 550 (&rwlock->__data.__readers, &r, + 551 ((r ^ PTHREAD_RWLOCK_WRLOCKED) + 552 ^ ((r >> PTHREAD_RWLOCK_READER_SHIFT) == 0 ? 0 + 553 : PTHREAD_RWLOCK_WRPHASE)))) + 554 { + ... + 556 } + + We clear PTHREAD_RWLOCK_WRLOCKED, and if there are + no readers so we leave the lock in PTHRAD_RWLOCK_WRPHASE. + + Back in the read lock. + + The read lock adjusts __readres as above. + + 383 while ((r & PTHREAD_RWLOCK_WRPHASE) != 0 + 384 && (r & PTHREAD_RWLOCK_WRLOCKED) == 0) + 385 { + ... + 390 if (atomic_compare_exchange_weak_acquire (&rwlock->__data.__readers, &r, + 391 r ^ PTHREAD_RWLOCK_WRPHASE)) + 392 { + + And then attemps to start the read phase. + + Assume there happens to be a tryrdlock at this point, noting + that PTHREAD_RWLOCK_WRLOCKED is clear, and PTHREAD_RWLOCK_WRPHASE + is 1. So the try lock attemps to start the read phase. + + In __pthread_rwlock_tryrdlock: + + 44 if ((r & PTHREAD_RWLOCK_WRPHASE) == 0) + 45 { + ... + 49 if (((r & PTHREAD_RWLOCK_WRLOCKED) != 0) + 50 && (rwlock->__data.__flags + 51 == PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP)) + 52 return EBUSY; + 53 rnew = r + (1 << PTHREAD_RWLOCK_READER_SHIFT); + 54 } + ... + 89 while (!atomic_compare_exchange_weak_acquire (&rwlock->__data.__readers, + 90 &r, rnew)); + + And succeeds. + + Back in the write unlock: + + 557 if ((r >> PTHREAD_RWLOCK_READER_SHIFT) != 0) + 558 { + ... + 563 if ((atomic_exchange_relaxed (&rwlock->__data.__wrphase_futex, 0) + 564 & PTHREAD_RWLOCK_FUTEX_USED) != 0) + 565 futex_wake (&rwlock->__data.__wrphase_futex, INT_MAX, private); + 566 } + + We note that PTHREAD_RWLOCK_FUTEX_USED is non-zero + and don't wake anyone. This is OK because we handed + over to the trylock. It will be the trylock's responsibility + to wake any waiters. + + Back in the read lock: + + The read lock fails to install PTHRAD_REWLOCK_WRPHASE as 0 because + the __readers value was adjusted by the trylock, and so it falls through + to waiting on the lock for explicit handover from either a new writer + or a new reader. + + 448 int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex, + 449 1 | PTHREAD_RWLOCK_FUTEX_USED, + 450 abstime, private); + + We use PTHREAD_RWLOCK_FUTEX_USED to indicate the futex + is in use. + + At this point we have readers waiting on the read lock + to unlock. The wrlock is done. The trylock is finishing + the installation of the read phase. + + 92 if ((r & PTHREAD_RWLOCK_WRPHASE) != 0) + 93 { + ... + 105 atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 0); + 106 } + + The trylock does note that we were the one that + installed the read phase, but the comments are not + correct, the execution ordering above shows that + readers might indeed be waiting, and they are. + + The atomic_store_relaxed throws away PTHREAD_RWLOCK_FUTEX_USED, + and the waiting reader is never worken becuase as noted + above it is conditional on the futex being used. + + The solution is for the trylock thread to inspect + PTHREAD_RWLOCK_FUTEX_USED and wake the waiting readers. + + --- Analysis of pthread_rwlock_trywrlock() stall --- + + A write lock begins to execute, takes the write lock, + and then releases the lock... + + In pthread_rwlock_wrunlock(): + + 547 unsigned int r = atomic_load_relaxed (&rwlock->__data.__readers); + ... + 549 while (!atomic_compare_exchange_weak_release + 550 (&rwlock->__data.__readers, &r, + 551 ((r ^ PTHREAD_RWLOCK_WRLOCKED) + 552 ^ ((r >> PTHREAD_RWLOCK_READER_SHIFT) == 0 ? 0 + 553 : PTHREAD_RWLOCK_WRPHASE)))) + 554 { + ... + 556 } + + ... leaving it in the write phase with zero readers + (the case where we leave the write phase in place + during a write unlock). + + A write trylock begins to execute. + + In __pthread_rwlock_trywrlock: + + 40 while (((r & PTHREAD_RWLOCK_WRLOCKED) == 0) + 41 && (((r >> PTHREAD_RWLOCK_READER_SHIFT) == 0) + 42 || (prefer_writer && ((r & PTHREAD_RWLOCK_WRPHASE) != 0)))) + 43 { + + The lock is not locked. + + There are no readers. + + 45 if (atomic_compare_exchange_weak_acquire ( + 46 &rwlock->__data.__readers, &r, + 47 r | PTHREAD_RWLOCK_WRPHASE | PTHREAD_RWLOCK_WRLOCKED)) + + We atomically install the write phase and we take the + exclusive write lock. + + 48 { + 49 atomic_store_relaxed (&rwlock->__data.__writers_futex, 1); + + We get this far. + + A reader lock begins to execute. + + In pthread_rwlock_rdlock: + + 437 for (;;) + 438 { + 439 while (((wpf = atomic_load_relaxed (&rwlock->__data.__wrphase_futex)) + 440 | PTHREAD_RWLOCK_FUTEX_USED) == (1 | PTHREAD_RWLOCK_FUTEX_USED)) + 441 { + 442 int private = __pthread_rwlock_get_private (rwlock); + 443 if (((wpf & PTHREAD_RWLOCK_FUTEX_USED) == 0) + 444 && (!atomic_compare_exchange_weak_relaxed + 445 (&rwlock->__data.__wrphase_futex, + 446 &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED))) + 447 continue; + 448 int err = futex_abstimed_wait (&rwlock->__data.__wrphase_futex, + 449 1 | PTHREAD_RWLOCK_FUTEX_USED, + 450 abstime, private); + + We are in a write phase, so the while() on line 439 is true. + + The value of wpf does not have PTHREAD_RWLOCK_FUTEX_USED set + since this is the first reader to lock. + + The atomic operation sets wpf with PTHREAD_RELOCK_FUTEX_USED + on the expectation that this reader will be woken during + the handoff. + + Back in pthread_rwlock_trywrlock: + + 50 atomic_store_relaxed (&rwlock->__data.__wrphase_futex, 1); + 51 atomic_store_relaxed (&rwlock->__data.__cur_writer, + 52 THREAD_GETMEM (THREAD_SELF, tid)); + 53 return 0; + 54 } + ... + 57 } + + We write 1 to __wrphase_futex discarding PTHREAD_RWLOCK_FUTEX_USED, + and so in the unlock we will not awaken the waiting reader. + + The solution to this is to realize that if we did not start the write + phase we need not write 1 or any other value to __wrphase_futex. + This ensures that any readers (which saw __wrphase_futex != 0) can + set PTHREAD_RWLOCK_FUTEX_USED and this can be used at unlock to + wake them. + + If we installed the write phase then all other readers are looping + here: + + In __pthread_rwlock_rdlock_full: + + 437 for (;;) + 438 { + 439 while (((wpf = atomic_load_relaxed (&rwlock->__data.__wrphase_futex)) + 440 | PTHREAD_RWLOCK_FUTEX_USED) == (1 | PTHREAD_RWLOCK_FUTEX_USED)) + 441 { + ... + 508 } + + waiting for the write phase to be installed or removed before they + can begin waiting on __wrphase_futex (part of the algorithm), or + taking a concurrent read lock, and thus we can safely write 1 to + __wrphase_futex. + + If we did not install the write phase then the readers may already + be waiting on the futex, the original writer wrote 1 to __wrphase_futex + as part of starting the write phase, and we cannot also write 1 + without loosing the PTHREAD_RWLOCK_FUTEX_USED bit. + + --- + + Summary for the pthread_rwlock_tryrdlock() stall: + + The stall is caused by pthread_rwlock_tryrdlock failing to check + that PTHREAD_RWLOCK_FUTEX_USED is set in the __wrphase_futex futex + and then waking the futex. + + The fix for bug 23844 ensures that waiters on __wrphase_futex are + correctly woken. Before the fix the test stalls as readers can + wait forever on __wrphase_futex. */ + +#include +#include +#include +#include +#include +#include + +/* We need only one lock to reproduce the issue. We will need multiple + threads to get the exact case where we have a read, try, and unlock + all interleaving to produce the case where the readers are waiting + and the try fails to wake them. */ +pthread_rwlock_t onelock; + +/* The number of threads is arbitrary but empirically chosen to have + enough threads that we see the condition where waiting readers are + not woken by a successful tryrdlock. */ +#define NTHREADS 32 + +_Atomic int do_exit; + +void * +run_loop (void *arg) +{ + int i = 0, ret; + while (!do_exit) + { + /* Arbitrarily choose if we are the writer or reader. Choose a + high enough ratio of readers to writers to make it likely + that readers block (and eventually are susceptable to + stalling). + + If we are a writer, take the write lock, and then unlock. + If we are a reader, try the lock, then lock, then unlock. */ + if ((i % 8) != 0) + xpthread_rwlock_wrlock (&onelock); + else + { + if ((ret = pthread_rwlock_tryrdlock (&onelock)) != 0) + { + if (ret == EBUSY) + xpthread_rwlock_rdlock (&onelock); + else + exit (EXIT_FAILURE); + } + } + /* Thread does some work and then unlocks. */ + xpthread_rwlock_unlock (&onelock); + i++; + } + return NULL; +} + +int +do_test (void) +{ + int i; + pthread_t tids[NTHREADS]; + xpthread_rwlock_init (&onelock, NULL); + for (i = 0; i < NTHREADS; i++) + tids[i] = xpthread_create (NULL, run_loop, NULL); + /* Run for some amount of time. Empirically speaking exercising + the stall via pthread_rwlock_tryrdlock is much harder, and on + a 3.5GHz 4 core x86_64 VM system it takes somewhere around + 20-200s to stall, approaching 100% stall past 200s. We can't + wait that long for a regression test so we just test for 20s, + and expect the stall to happen with a 5-10% chance (enough for + developers to see). */ + sleep (20); + /* Then exit. */ + printf ("INFO: Exiting...\n"); + do_exit = 1; + /* If any readers stalled then we will timeout waiting for them. */ + for (i = 0; i < NTHREADS; i++) + xpthread_join (tids[i]); + printf ("INFO: Done.\n"); + xpthread_rwlock_destroy (&onelock); + printf ("PASS: No pthread_rwlock_tryrdlock stalls detected.\n"); + return 0; +} + +#define TIMEOUT 30 +#include diff --git a/nptl/tst-rwlock-trywrlock-stall.c b/nptl/tst-rwlock-trywrlock-stall.c new file mode 100644 index 0000000000..14d27cbcbc --- /dev/null +++ b/nptl/tst-rwlock-trywrlock-stall.c @@ -0,0 +1,108 @@ +/* Bug 23844: Test for pthread_rwlock_trywrlock stalls. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* For a full analysis see comments in tst-rwlock-tryrdlock-stall.c. + + Summary for the pthread_rwlock_trywrlock() stall: + + The stall is caused by pthread_rwlock_trywrlock setting + __wrphase_futex futex to 1 and loosing the + PTHREAD_RWLOCK_FUTEX_USED bit. + + The fix for bug 23844 ensures that waiters on __wrphase_futex are + correctly woken. Before the fix the test stalls as readers can + wait forever on __wrphase_futex. */ + +#include +#include +#include +#include +#include +#include + +/* We need only one lock to reproduce the issue. We will need multiple + threads to get the exact case where we have a read, try, and unlock + all interleaving to produce the case where the readers are waiting + and the try clears the PTHREAD_RWLOCK_FUTEX_USED bit and a + subsequent unlock fails to wake them. */ +pthread_rwlock_t onelock; + +/* The number of threads is arbitrary but empirically chosen to have + enough threads that we see the condition where waiting readers are + not woken by a successful unlock. */ +#define NTHREADS 32 + +_Atomic int do_exit; + +void * +run_loop (void *arg) +{ + int i = 0, ret; + while (!do_exit) + { + /* Arbitrarily choose if we are the writer or reader. Choose a + high enough ratio of readers to writers to make it likely + that readers block (and eventually are susceptable to + stalling). + + If we are a writer, take the write lock, and then unlock. + If we are a reader, try the lock, then lock, then unlock. */ + if ((i % 8) != 0) + { + if ((ret = pthread_rwlock_trywrlock (&onelock)) != 0) + { + if (ret == EBUSY) + xpthread_rwlock_wrlock (&onelock); + else + exit (EXIT_FAILURE); + } + } + else + xpthread_rwlock_rdlock (&onelock); + /* Thread does some work and then unlocks. */ + xpthread_rwlock_unlock (&onelock); + i++; + } + return NULL; +} + +int +do_test (void) +{ + int i; + pthread_t tids[NTHREADS]; + xpthread_rwlock_init (&onelock, NULL); + for (i = 0; i < NTHREADS; i++) + tids[i] = xpthread_create (NULL, run_loop, NULL); + /* Run for some amount of time. The pthread_rwlock_tryrwlock stall + is very easy to trigger and happens in seconds under the test + conditions. */ + sleep (10); + /* Then exit. */ + printf ("INFO: Exiting...\n"); + do_exit = 1; + /* If any readers stalled then we will timeout waiting for them. */ + for (i = 0; i < NTHREADS; i++) + xpthread_join (tids[i]); + printf ("INFO: Done.\n"); + xpthread_rwlock_destroy (&onelock); + printf ("PASS: No pthread_rwlock_tryrwlock stalls detected.\n"); + return 0; +} + +#include diff --git a/support/Makefile b/support/Makefile index 432cf2fe6c..c15b93647c 100644 --- a/support/Makefile +++ b/support/Makefile @@ -129,6 +129,7 @@ libsupport-routines = \ xpthread_mutexattr_settype \ xpthread_once \ xpthread_rwlock_init \ + xpthread_rwlock_destroy \ xpthread_rwlock_rdlock \ xpthread_rwlock_unlock \ xpthread_rwlock_wrlock \ diff --git a/support/xpthread_rwlock_destroy.c b/support/xpthread_rwlock_destroy.c new file mode 100644 index 0000000000..6d6e953569 --- /dev/null +++ b/support/xpthread_rwlock_destroy.c @@ -0,0 +1,26 @@ +/* pthread_rwlock_destroy with error checking. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +void +xpthread_rwlock_destroy (pthread_rwlock_t *rwlock) +{ + xpthread_check_return ("pthread_rwlock_destroy", + pthread_rwlock_destroy (rwlock)); +} diff --git a/support/xthread.h b/support/xthread.h index 47c23235f3..9fe1f68b3b 100644 --- a/support/xthread.h +++ b/support/xthread.h @@ -84,6 +84,7 @@ void xpthread_rwlockattr_setkind_np (pthread_rwlockattr_t *attr, int pref); void xpthread_rwlock_wrlock (pthread_rwlock_t *rwlock); void xpthread_rwlock_rdlock (pthread_rwlock_t *rwlock); void xpthread_rwlock_unlock (pthread_rwlock_t *rwlock); +void xpthread_rwlock_destroy (pthread_rwlock_t *rwlock); __END_DECLS commit 726a78867b3144e9b9da10197bcf59bde3d8b2a4 Author: H.J. Lu Date: Mon Feb 4 08:55:52 2019 -0800 x86-64 memcmp: Use unsigned Jcc instructions on size [BZ #24155] Since the size argument is unsigned. we should use unsigned Jcc instructions, instead of signed, to check size. Tested on x86-64 and x32, with and without --disable-multi-arch. [BZ #24155] CVE-2019-7309 * NEWS: Updated for CVE-2019-7309. * sysdeps/x86_64/memcmp.S: Use RDX_LP for size. Clear the upper 32 bits of RDX register for x32. Use unsigned Jcc instructions, instead of signed. * sysdeps/x86_64/x32/Makefile (tests): Add tst-size_t-memcmp-2. * sysdeps/x86_64/x32/tst-size_t-memcmp-2.c: New test. (cherry picked from commit 3f635fb43389b54f682fc9ed2acc0b2aaf4a923d) diff --git a/ChangeLog b/ChangeLog index adb4e719a6..e0969afd80 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2019-02-04 H.J. Lu + + [BZ #24155] + CVE-2019-7309 + * NEWS: Updated for CVE-2019-7309. + * sysdeps/x86_64/memcmp.S: Use RDX_LP for size. Clear the + upper 32 bits of RDX register for x32. Use unsigned Jcc + instructions, instead of signed. + * sysdeps/x86_64/x32/Makefile (tests): Add tst-size_t-memcmp-2. + * sysdeps/x86_64/x32/tst-size_t-memcmp-2.c: New test. + 2019-01-31 Carlos O'Donell Torvald Riegel Rik Prohaska diff --git a/NEWS b/NEWS index 912a9bdc0f..1751ed118a 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,23 @@ See the end for copying conditions. Please send GNU C library bug reports via using `glibc' in the "product" field. +Version 2.29.1 + +The following bugs are resolved with this release: + + [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) + +Security related changes: + + CVE-2019-7309: x86-64 memcmp used signed Jcc instructions to check + size. For x86-64, memcmp on an object size larger than SSIZE_MAX + has undefined behavior. On x32, the size_t argument may be passed + in the lower 32 bits of the 64-bit RDX register with non-zero upper + 32 bits. When it happened with the sign bit of RDX register set, + memcmp gave the wrong result since it treated the size argument as + zero. Reported by H.J. Lu. + + Version 2.29 Major new features: diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S index 1fc487caa5..1322bb3b92 100644 --- a/sysdeps/x86_64/memcmp.S +++ b/sysdeps/x86_64/memcmp.S @@ -21,14 +21,18 @@ .text ENTRY (memcmp) - test %rdx, %rdx +#ifdef __ILP32__ + /* Clear the upper 32 bits. */ + movl %edx, %edx +#endif + test %RDX_LP, %RDX_LP jz L(finz) cmpq $1, %rdx - jle L(finr1b) + jbe L(finr1b) subq %rdi, %rsi movq %rdx, %r10 cmpq $32, %r10 - jge L(gt32) + jae L(gt32) /* Handle small chunks and last block of less than 32 bytes. */ L(small): testq $1, %r10 @@ -156,7 +160,7 @@ L(A32): movq %r11, %r10 andq $-32, %r10 cmpq %r10, %rdi - jge L(mt16) + jae L(mt16) /* Pre-unroll to be ready for unrolled 64B loop. */ testq $32, %rdi jz L(A64) @@ -178,7 +182,7 @@ L(A64): movq %r11, %r10 andq $-64, %r10 cmpq %r10, %rdi - jge L(mt32) + jae L(mt32) L(A64main): movdqu (%rdi,%rsi), %xmm0 @@ -216,7 +220,7 @@ L(mt32): movq %r11, %r10 andq $-32, %r10 cmpq %r10, %rdi - jge L(mt16) + jae L(mt16) L(A32main): movdqu (%rdi,%rsi), %xmm0 @@ -254,7 +258,7 @@ L(ATR): movq %r11, %r10 andq $-32, %r10 cmpq %r10, %rdi - jge L(mt16) + jae L(mt16) testq $16, %rdi jz L(ATR32) @@ -325,7 +329,7 @@ L(ATR64main): movq %r11, %r10 andq $-32, %r10 cmpq %r10, %rdi - jge L(mt16) + jae L(mt16) L(ATR32res): movdqa (%rdi,%rsi), %xmm0 diff --git a/sysdeps/x86_64/x32/Makefile b/sysdeps/x86_64/x32/Makefile index 1557724b0c..8748956563 100644 --- a/sysdeps/x86_64/x32/Makefile +++ b/sysdeps/x86_64/x32/Makefile @@ -8,7 +8,8 @@ endif ifeq ($(subdir),string) tests += tst-size_t-memchr tst-size_t-memcmp tst-size_t-memcpy \ tst-size_t-memrchr tst-size_t-memset tst-size_t-strncasecmp \ - tst-size_t-strncmp tst-size_t-strncpy tst-size_t-strnlen + tst-size_t-strncmp tst-size_t-strncpy tst-size_t-strnlen \ + tst-size_t-memcmp-2 endif ifeq ($(subdir),wcsmbs) diff --git a/sysdeps/x86_64/x32/tst-size_t-memcmp-2.c b/sysdeps/x86_64/x32/tst-size_t-memcmp-2.c new file mode 100644 index 0000000000..d8ae1a0813 --- /dev/null +++ b/sysdeps/x86_64/x32/tst-size_t-memcmp-2.c @@ -0,0 +1,79 @@ +/* Test memcmp with size_t in the lower 32 bits of 64-bit register. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#define TEST_MAIN +#ifdef WIDE +# define TEST_NAME "wmemcmp" +#else +# define TEST_NAME "memcmp" +#endif + +#include "test-size_t.h" + +#ifdef WIDE +# include +# include + +# define MEMCMP wmemcmp +# define CHAR wchar_t +#else +# define MEMCMP memcmp +# define CHAR char +#endif + +IMPL (MEMCMP, 1) + +typedef int (*proto_t) (const CHAR *, const CHAR *, size_t); + +static int +__attribute__ ((noinline, noclone)) +do_memcmp (parameter_t a, parameter_t b) +{ + return CALL (&b, a.p, b.p, a.len); +} + +static int +test_main (void) +{ + test_init (); + + parameter_t dest = { { page_size / sizeof (CHAR) }, buf1 }; + parameter_t src = { { 0 }, buf2 }; + + memcpy (buf1, buf2, page_size); + + CHAR *p = (CHAR *) buf1; + p[page_size / sizeof (CHAR) - 1] = (CHAR) 1; + + int ret = 0; + FOR_EACH_IMPL (impl, 0) + { + src.fn = impl->fn; + int res = do_memcmp (dest, src); + if (res >= 0) + { + error (0, 0, "Wrong result in function %s: %i >= 0", + impl->name, res); + ret = 1; + } + } + + return ret ? EXIT_FAILURE : EXIT_SUCCESS; +} + +#include commit 2de15ac95713a238dc258eb8977ecdfca811fc19 Author: Florian Weimer Date: Tue Feb 5 13:49:02 2019 +0100 arm: Use "nr" constraint for Systemtap probes [BZ #24164] With the default "nor" constraint, current GCC will use the "o" constraint for constants, after emitting the constant to memory. That results in unparseable Systemtap probe notes such as "-4@.L1052". Removing the "o" alternative and using "nr" instead avoids this. (cherry picked from commit f1ac7455831546e5dca0ed98fe8af2686fae7ce6) diff --git a/ChangeLog b/ChangeLog index e0969afd80..9517ff9e82 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2019-02-05 Florian Weimer + + [BZ #24164] + arm: Use "nr" constraint for Systemtap probes, to avoid the + compiler using memory operands for constants, due to the "o" + alternative in the default "nor" constraint. + * include/stap-probe.h [USE_STAP_PROBE]: Include + + * sysdeps/generic/stap-probe-machine.h: New file. + * sysdeps/arm/stap-probe-machine.h: Likewise. + 2019-02-04 H.J. Lu [BZ #24155] diff --git a/NEWS b/NEWS index 1751ed118a..1f0fc4b3cb 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,7 @@ Version 2.29.1 The following bugs are resolved with this release: [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) + [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm Security related changes: diff --git a/include/stap-probe.h b/include/stap-probe.h index c53dd86592..8c26292edd 100644 --- a/include/stap-probe.h +++ b/include/stap-probe.h @@ -21,6 +21,7 @@ #ifdef USE_STAP_PROBE +# include # include /* Our code uses one macro LIBC_PROBE (name, n, arg1, ..., argn). diff --git a/sysdeps/arm/stap-probe-machine.h b/sysdeps/arm/stap-probe-machine.h new file mode 100644 index 0000000000..d27ca22040 --- /dev/null +++ b/sysdeps/arm/stap-probe-machine.h @@ -0,0 +1,22 @@ +/* Macros for customizing Systemtap . Arm version. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* The default "nor" constraint produces unparseable memory references + for constants. Omit the problematic "o" constraint. See bug 24164 + and GCC PR 89146. */ +#define STAP_SDT_ARG_CONSTRAINT nr diff --git a/sysdeps/generic/stap-probe-machine.h b/sysdeps/generic/stap-probe-machine.h new file mode 100644 index 0000000000..2e5790c3b2 --- /dev/null +++ b/sysdeps/generic/stap-probe-machine.h @@ -0,0 +1,19 @@ +/* Macros for customizing Systemtap . Generic version. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* By default, there are no customizations. */ commit 44113a8ba24af23d7bbb174f9087a6b83a76289a Author: Stefan Liebler Date: Thu Feb 7 15:18:36 2019 +0100 Add compiler barriers around modifications of the robust mutex list for pthread_mutex_trylock. [BZ #24180] While debugging a kernel warning, Thomas Gleixner, Sebastian Sewior and Heiko Carstens found a bug in pthread_mutex_trylock due to misordered instructions: 140: a5 1b 00 01 oill %r1,1 144: e5 48 a0 f0 00 00 mvghi 240(%r10),0 <--- THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); 14a: e3 10 a0 e0 00 24 stg %r1,224(%r10) <--- last THREAD_SETMEM of ENQUEUE_MUTEX_PI vs (with compiler barriers): 140: a5 1b 00 01 oill %r1,1 144: e3 10 a0 e0 00 24 stg %r1,224(%r10) 14a: e5 48 a0 f0 00 00 mvghi 240(%r10),0 Please have a look at the discussion: "Re: WARN_ON_ONCE(!new_owner) within wake_futex_pi() triggerede" (https://lore.kernel.org/lkml/20190202112006.GB3381@osiris/) This patch is introducing the same compiler barriers and comments for pthread_mutex_trylock as introduced for pthread_mutex_lock and pthread_mutex_timedlock by commit 8f9450a0b7a9e78267e8ae1ab1000ebca08e473e "Add compiler barriers around modifications of the robust mutex list." ChangeLog: [BZ #24180] * nptl/pthread_mutex_trylock.c (__pthread_mutex_trylock): Add compiler barriers and comments. (cherry picked from commit 823624bdc47f1f80109c9c52dee7939b9386d708) diff --git a/ChangeLog b/ChangeLog index 9517ff9e82..7a8895bcc6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2019-02-07 Stefan Liebler + + [BZ #24180] + * nptl/pthread_mutex_trylock.c (__pthread_mutex_trylock): + Add compiler barriers and comments. + 2019-02-05 Florian Weimer [BZ #24164] diff --git a/nptl/pthread_mutex_trylock.c b/nptl/pthread_mutex_trylock.c index 8fe43b8f0f..bf2869eca2 100644 --- a/nptl/pthread_mutex_trylock.c +++ b/nptl/pthread_mutex_trylock.c @@ -94,6 +94,9 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) case PTHREAD_MUTEX_ROBUST_ADAPTIVE_NP: THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, &mutex->__data.__list.__next); + /* We need to set op_pending before starting the operation. Also + see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); oldval = mutex->__data.__lock; do @@ -119,7 +122,12 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) /* But it is inconsistent unless marked otherwise. */ mutex->__data.__owner = PTHREAD_MUTEX_INCONSISTENT; + /* We must not enqueue the mutex before we have acquired it. + Also see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); ENQUEUE_MUTEX (mutex); + /* We need to clear op_pending after we enqueue the mutex. */ + __asm ("" ::: "memory"); THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); /* Note that we deliberately exist here. If we fall @@ -135,6 +143,8 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) int kind = PTHREAD_MUTEX_TYPE (mutex); if (kind == PTHREAD_MUTEX_ROBUST_ERRORCHECK_NP) { + /* We do not need to ensure ordering wrt another memory + access. Also see comments at ENQUEUE_MUTEX. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return EDEADLK; @@ -142,6 +152,8 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) if (kind == PTHREAD_MUTEX_ROBUST_RECURSIVE_NP) { + /* We do not need to ensure ordering wrt another memory + access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); @@ -160,6 +172,9 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) id, 0); if (oldval != 0 && (oldval & FUTEX_OWNER_DIED) == 0) { + /* We haven't acquired the lock as it is already acquired by + another owner. We do not need to ensure ordering wrt another + memory access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return EBUSY; @@ -173,13 +188,20 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) if (oldval == id) lll_unlock (mutex->__data.__lock, PTHREAD_ROBUST_MUTEX_PSHARED (mutex)); + /* FIXME This violates the mutex destruction requirements. See + __pthread_mutex_unlock_full. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return ENOTRECOVERABLE; } } while ((oldval & FUTEX_OWNER_DIED) != 0); + /* We must not enqueue the mutex before we have acquired it. + Also see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); ENQUEUE_MUTEX (mutex); + /* We need to clear op_pending after we enqueue the mutex. */ + __asm ("" ::: "memory"); THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); mutex->__data.__owner = id; @@ -211,10 +233,15 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) } if (robust) - /* Note: robust PI futexes are signaled by setting bit 0. */ - THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, - (void *) (((uintptr_t) &mutex->__data.__list.__next) - | 1)); + { + /* Note: robust PI futexes are signaled by setting bit 0. */ + THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, + (void *) (((uintptr_t) &mutex->__data.__list.__next) + | 1)); + /* We need to set op_pending before starting the operation. Also + see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); + } oldval = mutex->__data.__lock; @@ -223,12 +250,16 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) { if (kind == PTHREAD_MUTEX_ERRORCHECK_NP) { + /* We do not need to ensure ordering wrt another memory + access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return EDEADLK; } if (kind == PTHREAD_MUTEX_RECURSIVE_NP) { + /* We do not need to ensure ordering wrt another memory + access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); /* Just bump the counter. */ @@ -250,6 +281,9 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) { if ((oldval & FUTEX_OWNER_DIED) == 0) { + /* We haven't acquired the lock as it is already acquired by + another owner. We do not need to ensure ordering wrt another + memory access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return EBUSY; @@ -270,6 +304,9 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) if (INTERNAL_SYSCALL_ERROR_P (e, __err) && INTERNAL_SYSCALL_ERRNO (e, __err) == EWOULDBLOCK) { + /* The kernel has not yet finished the mutex owner death. + We do not need to ensure ordering wrt another memory + access. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return EBUSY; @@ -287,7 +324,12 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) /* But it is inconsistent unless marked otherwise. */ mutex->__data.__owner = PTHREAD_MUTEX_INCONSISTENT; + /* We must not enqueue the mutex before we have acquired it. + Also see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); ENQUEUE_MUTEX (mutex); + /* We need to clear op_pending after we enqueue the mutex. */ + __asm ("" ::: "memory"); THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); /* Note that we deliberately exit here. If we fall @@ -310,13 +352,20 @@ __pthread_mutex_trylock (pthread_mutex_t *mutex) PTHREAD_ROBUST_MUTEX_PSHARED (mutex)), 0, 0); + /* To the kernel, this will be visible after the kernel has + acquired the mutex in the syscall. */ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); return ENOTRECOVERABLE; } if (robust) { + /* We must not enqueue the mutex before we have acquired it. + Also see comments at ENQUEUE_MUTEX. */ + __asm ("" ::: "memory"); ENQUEUE_MUTEX_PI (mutex); + /* We need to clear op_pending after we enqueue the mutex. */ + __asm ("" ::: "memory"); THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL); } commit c096b008d2671028c21ac8cf01f18a2083e73c44 Author: Florian Weimer Date: Fri Feb 8 12:54:41 2019 +0100 nptl: Avoid fork handler lock for async-signal-safe fork [BZ #24161] Commit 27761a1042daf01987e7d79636d0c41511c6df3c ("Refactor atfork handlers") introduced a lock, atfork_lock, around fork handler list accesses. It turns out that this lock occasionally results in self-deadlocks in malloc/tst-mallocfork2: (gdb) bt #0 __lll_lock_wait_private () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:63 #1 0x00007f160c6f927a in __run_fork_handlers (who=(unknown: 209394016), who@entry=atfork_run_prepare) at register-atfork.c:116 #2 0x00007f160c6b7897 in __libc_fork () at ../sysdeps/nptl/fork.c:58 #3 0x00000000004027d6 in sigusr1_handler (signo=) at tst-mallocfork2.c:80 #4 sigusr1_handler (signo=) at tst-mallocfork2.c:64 #5 #6 0x00007f160c6f92e4 in __run_fork_handlers (who=who@entry=atfork_run_parent) at register-atfork.c:136 #7 0x00007f160c6b79a2 in __libc_fork () at ../sysdeps/nptl/fork.c:152 #8 0x0000000000402567 in do_test () at tst-mallocfork2.c:156 #9 0x0000000000402dd2 in support_test_main (argc=1, argv=0x7ffc81ef1ab0, config=config@entry=0x7ffc81ef1970) at support_test_main.c:350 #10 0x0000000000402362 in main (argc=, argv=) at ../support/test-driver.c:168 If no locking happens in the single-threaded case (where fork is expected to be async-signal-safe), this deadlock is avoided. (pthread_atfork is not required to be async-signal-safe, so a fork call from a signal handler interrupting pthread_atfork is not a problem.) (cherry picked from commit 669ff911e2571f74a2668493e326ac9a505776bd) diff --git a/ChangeLog b/ChangeLog index 7a8895bcc6..d363be4620 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2019-02-08 Florian Weimer + + [BZ #24161] + * sysdeps/nptl/fork.h (__run_fork_handlers): Add multiple_threads + argument. + * nptl/register-atfork.c (__run_fork_handlers): Only perform + locking if the new do_locking argument is true. + * sysdeps/nptl/fork.c (__libc_fork): Pass multiple_threads to + __run_fork_handlers. + 2019-02-07 Stefan Liebler [BZ #24180] diff --git a/NEWS b/NEWS index 1f0fc4b3cb..dbcdd48502 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,7 @@ The following bugs are resolved with this release: [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm + [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 Security related changes: diff --git a/nptl/register-atfork.c b/nptl/register-atfork.c index bc797b761a..80a1becb5f 100644 --- a/nptl/register-atfork.c +++ b/nptl/register-atfork.c @@ -107,13 +107,14 @@ __unregister_atfork (void *dso_handle) } void -__run_fork_handlers (enum __run_fork_handler_type who) +__run_fork_handlers (enum __run_fork_handler_type who, _Bool do_locking) { struct fork_handler *runp; if (who == atfork_run_prepare) { - lll_lock (atfork_lock, LLL_PRIVATE); + if (do_locking) + lll_lock (atfork_lock, LLL_PRIVATE); size_t sl = fork_handler_list_size (&fork_handlers); for (size_t i = sl; i > 0; i--) { @@ -133,7 +134,8 @@ __run_fork_handlers (enum __run_fork_handler_type who) else if (who == atfork_run_parent && runp->parent_handler) runp->parent_handler (); } - lll_unlock (atfork_lock, LLL_PRIVATE); + if (do_locking) + lll_unlock (atfork_lock, LLL_PRIVATE); } } diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c index bd68f18b45..14b69a6f89 100644 --- a/sysdeps/nptl/fork.c +++ b/sysdeps/nptl/fork.c @@ -55,7 +55,7 @@ __libc_fork (void) but our current fork implementation is not. */ bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads); - __run_fork_handlers (atfork_run_prepare); + __run_fork_handlers (atfork_run_prepare, multiple_threads); /* If we are not running multiple threads, we do not have to preserve lock state. If fork runs from a signal handler, only @@ -134,7 +134,7 @@ __libc_fork (void) __rtld_lock_initialize (GL(dl_load_lock)); /* Run the handlers registered for the child. */ - __run_fork_handlers (atfork_run_child); + __run_fork_handlers (atfork_run_child, multiple_threads); } else { @@ -149,7 +149,7 @@ __libc_fork (void) } /* Run the handlers registered for the parent. */ - __run_fork_handlers (atfork_run_parent); + __run_fork_handlers (atfork_run_parent, multiple_threads); } return pid; diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h index a1c3b26b68..99ed76034b 100644 --- a/sysdeps/nptl/fork.h +++ b/sysdeps/nptl/fork.h @@ -52,9 +52,11 @@ enum __run_fork_handler_type - atfork_run_child: run all the CHILD_HANDLER and unlocks the internal lock. - atfork_run_parent: run all the PARENT_HANDLER and unlocks the internal - lock. */ -extern void __run_fork_handlers (enum __run_fork_handler_type who) - attribute_hidden; + lock. + + Perform locking only if DO_LOCKING. */ +extern void __run_fork_handlers (enum __run_fork_handler_type who, + _Bool do_locking) attribute_hidden; /* C library side function to register new fork handlers. */ extern int __register_atfork (void (*__prepare) (void), commit 067fc32968b601493f4b247a3ac00caeea3f3d61 Author: Florian Weimer Date: Fri Feb 15 21:27:01 2019 +0100 nptl: Fix invalid Systemtap probe in pthread_join [BZ #24211] After commit f1ac7455831546e5dca0ed98fe8af2686fae7ce6 ("arm: Use "nr" constraint for Systemtap probes [BZ #24164]"), we load pd->result into a register in the probe below: /* Free the TCB. */ __free_tcb (pd); } else pd->joinid = NULL; LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result); However, at this point, the thread descriptor has been freed. If the thread stack does not fit into the thread stack cache, the memory will have been unmapped, and the program will crash in the probe. (cherry picked from commit bc10e22c90e42613bd5dafb77b80a9ea1759dd1b) diff --git a/ChangeLog b/ChangeLog index d363be4620..a6a0ce19ed 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2019-02-15 Florian Weimer + + [BZ #24211] + * nptl/pthread_join_common.c (__pthread_timedjoin_ex): Do not read + pd->result after the thread descriptor has been freed. + 2019-02-08 Florian Weimer [BZ #24161] diff --git a/NEWS b/NEWS index dbcdd48502..340e06d0f4 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,7 @@ The following bugs are resolved with this release: [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 + [24211] Use-after-free in Systemtap probe in pthread_join Security related changes: diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index ecb78ffba5..366feb376b 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -86,6 +86,7 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return, pthread_cleanup_pop (0); } + void *pd_result = pd->result; if (__glibc_likely (result == 0)) { /* We mark the thread as terminated and as joined. */ @@ -93,7 +94,7 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return, /* Store the return value if the caller is interested. */ if (thread_return != NULL) - *thread_return = pd->result; + *thread_return = pd_result; /* Free the TCB. */ __free_tcb (pd); @@ -101,7 +102,7 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return, else pd->joinid = NULL; - LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result); + LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd_result); return result; } commit bc6f839fb4066be83272c735e662850af2595777 Author: Stefan Liebler Date: Wed Mar 13 10:45:35 2019 +0100 Fix output of LD_SHOW_AUXV=1. Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e the output was corrupted on some platforms as _dl_procinfo was called for every auxv entry and on some architectures like s390 all entries were represented as "AT_HWCAP". This patch is removing the condition and let _dl_procinfo decide if an entry is printed in a platform specific or generic way. This patch also adjusts all _dl_procinfo implementations which assumed that they are only called for AT_HWCAP or AT_HWCAP2. They are now just returning a non-zero-value for entries which are not handled platform specifc. ChangeLog: * elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always call _dl_procinfo. * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo): Ignore types other than AT_HWCAP. * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise. * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): Likewise. * sysdeps/powerpc/dl-procinfo.h (_dl_procinfo): Adjust comment in the case of falling back to generic output mechanism. * sysdeps/unix/sysv/linux/arm/dl-procinfo.h (_dl_procinfo): Likewise. (cherry picked from commit 7c6513082b787a7d36ab7d75720b48f8a216089c) Conflicts: ChangeLog diff --git a/ChangeLog b/ChangeLog index a6a0ce19ed..90558e434c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2019-03-13 Stefan Liebler + + * elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always + call _dl_procinfo. + * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo): + Ignore types other than AT_HWCAP. + * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise. + * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): + Likewise. + * sysdeps/powerpc/dl-procinfo.h (_dl_procinfo): Adjust comment + in the case of falling back to generic output mechanism. + * sysdeps/unix/sysv/linux/arm/dl-procinfo.h (_dl_procinfo): + Likewise. + 2019-02-15 Florian Weimer [BZ #24211] diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c index 5f6c679a3f..5d19b100b2 100644 --- a/elf/dl-sysdep.c +++ b/elf/dl-sysdep.c @@ -328,14 +328,9 @@ _dl_show_auxv (void) assert (AT_NULL == 0); assert (AT_IGNORE == 1); - if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2 - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY) - { - /* These are handled in a special way per platform. */ - if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0) - continue; - } + /* Some entries are handled in a special way per platform. */ + if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0) + continue; if (idx < sizeof (auxvars) / sizeof (auxvars[0]) && auxvars[idx].form != unknown) diff --git a/sysdeps/powerpc/dl-procinfo.h b/sysdeps/powerpc/dl-procinfo.h index f542f7318f..dfc3b33a72 100644 --- a/sysdeps/powerpc/dl-procinfo.h +++ b/sysdeps/powerpc/dl-procinfo.h @@ -225,7 +225,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) break; } default: - /* This should not happen. */ + /* Fallback to generic output mechanism. */ return -1; } _dl_printf ("\n"); diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h index 282b8c5117..64ee267fc7 100644 --- a/sysdeps/sparc/dl-procinfo.h +++ b/sysdeps/sparc/dl-procinfo.h @@ -31,8 +31,8 @@ _dl_procinfo (unsigned int type, unsigned long int word) { int i; - /* Fallback to unknown output mechanism. */ - if (type == AT_HWCAP2) + /* Fallback to generic output mechanism. */ + if (type != AT_HWCAP) return -1; _dl_printf ("AT_HWCAP: "); diff --git a/sysdeps/unix/sysv/linux/arm/dl-procinfo.h b/sysdeps/unix/sysv/linux/arm/dl-procinfo.h index 66c00297b7..05c62c8687 100644 --- a/sysdeps/unix/sysv/linux/arm/dl-procinfo.h +++ b/sysdeps/unix/sysv/linux/arm/dl-procinfo.h @@ -67,7 +67,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) break; } default: - /* This should not happen. */ + /* Fallback to generic output mechanism. */ return -1; } _dl_printf ("\n"); diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h index 22b43431bc..0585cdaa9c 100644 --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h @@ -30,8 +30,8 @@ _dl_procinfo (unsigned int type, unsigned long int word) in the kernel sources. */ int i; - /* Fallback to unknown output mechanism. */ - if (type == AT_HWCAP2) + /* Fallback to generic output mechanism. */ + if (type != AT_HWCAP) return -1; _dl_printf ("AT_HWCAP: "); diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h index 19329a335b..d67fde368f 100644 --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h @@ -32,8 +32,8 @@ _dl_procinfo (unsigned int type, unsigned long int word) in the kernel sources. */ int i; - /* Fallback to unknown output mechanism. */ - if (type == AT_HWCAP2) + /* Fallback to generic output mechanism. */ + if (type != AT_HWCAP) return -1; _dl_printf ("AT_HWCAP: "); commit 4d0b1b0f61bfba034e9e76a1d76acc59c975238f Author: Paul Eggert Date: Mon Jan 21 11:08:13 2019 -0800 regex: fix read overrun [BZ #24114] Problem found by AddressSanitizer, reported by Hongxu Chen in: https://debbugs.gnu.org/34140 * posix/regexec.c (proceed_next_node): Do not read past end of input buffer. (cherry picked from commit 583dd860d5b833037175247230a328f0050dbfe9) diff --git a/ChangeLog b/ChangeLog index 90558e434c..fb88626efe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2019-01-31 Paul Eggert + + regex: fix read overrun [BZ #24114] + Problem found by AddressSanitizer, reported by Hongxu Chen in: + https://debbugs.gnu.org/34140 + * posix/regexec.c (proceed_next_node): + Do not read past end of input buffer. + 2019-03-13 Stefan Liebler * elf/dl-sysdep.c (_dl_show_auxv): Remove condition and always diff --git a/posix/regexec.c b/posix/regexec.c index 91d5a797b8..084b1222d9 100644 --- a/posix/regexec.c +++ b/posix/regexec.c @@ -1293,8 +1293,10 @@ proceed_next_node (const re_match_context_t *mctx, Idx nregs, regmatch_t *regs, else if (naccepted) { char *buf = (char *) re_string_get_buffer (&mctx->input); - if (memcmp (buf + regs[subexp_idx].rm_so, buf + *pidx, - naccepted) != 0) + if (mctx->input.valid_len - *pidx < naccepted + || (memcmp (buf + regs[subexp_idx].rm_so, buf + *pidx, + naccepted) + != 0)) return -1; } } commit 10dd17da710fd32aaf1f2187544d80064b8c4ee0 Author: Aurelien Jarno Date: Sat Mar 16 22:59:56 2019 +0100 Record CVE-2019-9169 in NEWS and ChangeLog [BZ #24114] (cherry picked from commit b626c5aa5d0673a9caa48fb79fba8bda237e6fa8) diff --git a/ChangeLog b/ChangeLog index fb88626efe..80413dd560 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 2019-01-31 Paul Eggert + CVE-2019-9169 regex: fix read overrun [BZ #24114] Problem found by AddressSanitizer, reported by Hongxu Chen in: https://debbugs.gnu.org/34140 diff --git a/NEWS b/NEWS index 340e06d0f4..271bf7a2cd 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,10 @@ Security related changes: memcmp gave the wrong result since it treated the size argument as zero. Reported by H.J. Lu. + CVE-2019-9169: Attempted case-insensitive regular-expression match + via proceed_next_node in posix/regexec.c leads to heap-based buffer + over-read. Reported by Hongxu Chen. + Version 2.29 commit 6eb48fe80cb6dd3ef536e86d005976d1c22b170e Author: Stefan Liebler Date: Thu Mar 21 09:14:26 2019 +0100 S390: Mark vx and vxe as important hwcap. This patch adds vx and vxe as important hwcaps which allows one to provide shared libraries tuned for platforms with non-vx/-vxe, vx or vxe. ChangeLog: * sysdeps/s390/dl-procinfo.h (HWCAP_IMPORTANT): Add HWCAP_S390_VX and HWCAP_S390_VXE. (cherry picked from commit 61f5e9470fb397a4c334938ac5a667427d9047df) Conflicts: ChangeLog diff --git a/ChangeLog b/ChangeLog index 80413dd560..7111aeb149 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2019-03-21 Stefan Liebler + + * sysdeps/s390/dl-procinfo.h (HWCAP_IMPORTANT): + Add HWCAP_S390_VX and HWCAP_S390_VXE. + 2019-01-31 Paul Eggert CVE-2019-9169 diff --git a/sysdeps/s390/dl-procinfo.h b/sysdeps/s390/dl-procinfo.h index b4b81fc70a..99697ae649 100644 --- a/sysdeps/s390/dl-procinfo.h +++ b/sysdeps/s390/dl-procinfo.h @@ -57,7 +57,8 @@ enum }; #define HWCAP_IMPORTANT (HWCAP_S390_ZARCH | HWCAP_S390_LDISP \ - | HWCAP_S390_EIMM | HWCAP_S390_DFP) + | HWCAP_S390_EIMM | HWCAP_S390_DFP \ + | HWCAP_S390_VX | HWCAP_S390_VXE) /* We cannot provide a general printing function. */ #define _dl_procinfo(type, word) -1 commit e28ad442e73b00ae2047d89c8cc7f9b2a0de5436 Author: TAMUKI Shoichi Date: Sat Mar 2 21:00:28 2019 +0900 ja_JP: Change the offset for Taisho gan-nen from 2 to 1 [BZ #24162] The offset in era-string format for Taisho gan-nen (1912) is currently defined as 2, but it should be 1. So fix it. "Gan-nen" means the 1st (origin) year, Taisho started on July 30, 1912. Reported-by: Morimitsu, Junji Reviewed-by: Rafal Luzynski ChangeLog: [BZ #24162] * localedata/locales/ja_JP (LC_TIME): Change the offset for Taisho gan-nen from 2 to 1. Problem reported by Morimitsu, Junji. (cherry picked from commit 31effacee2fc1b327bedc9a5fcb4b83f227c6539) diff --git a/ChangeLog b/ChangeLog index 7111aeb149..048ca9644c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2019-03-02 TAMUKI Shoichi + + [BZ #24162] + * localedata/locales/ja_JP (LC_TIME): Change the offset for Taisho + gan-nen from 2 to 1. Problem reported by Morimitsu, Junji. + 2019-03-21 Stefan Liebler * sysdeps/s390/dl-procinfo.h (HWCAP_IMPORTANT): diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP index 1fd2fee44b..9bfbb2bb9b 100644 --- a/localedata/locales/ja_JP +++ b/localedata/locales/ja_JP @@ -14951,7 +14951,7 @@ era "+:2:1990//01//01:+*::%EC%Ey";/ "+:2:1927//01//01:1989//01//07::%EC%Ey";/ "+:1:1926//12//25:1926//12//31::%EC";/ "+:2:1913//01//01:1926//12//24::%EC%Ey";/ - "+:2:1912//07//30:1912//12//31::%EC";/ + "+:1:1912//07//30:1912//12//31::%EC";/ "+:6:1873//01//01:1912//07//29::%EC%Ey";/ "+:1:0001//01//01:1872//12//31::%EC%Ey";/ "+:1:-0001//12//31:-*::%EC%Ey" commit 0941350c20a52447e53c5169354408e3db591f73 Author: TAMUKI Shoichi Date: Tue Apr 2 16:46:55 2019 +0900 ja_JP locale: Add entry for the new Japanese era [BZ #22964] The Japanese era name will be changed on May 1, 2019. The Japanese government made a preliminary announcement on April 1, 2019. The glibc ja_JP locale must be updated to include the new era name for strftime's alternative year format support. This is a minimal cherry pick of just the required locale changes. (cherry picked from commit 466afec30896585b60c2106df7a722a86247c9f3) diff --git a/ChangeLog b/ChangeLog index 048ca9644c..3b5d24cf67 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2019-04-02 TAMUKI Shoichi + + [BZ #22964] + * localedata/locales/ja_JP (LC_TIME): Add entry for the new Japanese + era. + 2019-03-02 TAMUKI Shoichi [BZ #24162] diff --git a/NEWS b/NEWS index 271bf7a2cd..703864ac75 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ using `glibc' in the "product" field. Version 2.29.1 +Major new features: + +* The entry for the new Japanese era has been added for ja_JP locale. + The following bugs are resolved with this release: [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP index 9bfbb2bb9b..c64aaaff55 100644 --- a/localedata/locales/ja_JP +++ b/localedata/locales/ja_JP @@ -14946,7 +14946,9 @@ am_pm "";"" t_fmt_ampm "%p%I%M%S" -era "+:2:1990//01//01:+*::%EC%Ey";/ +era "+:2:2020//01//01:+*::%EC%Ey";/ + "+:1:2019//05//01:2019//12//31::%EC";/ + "+:2:1990//01//01:2019//04//30::%EC%Ey";/ "+:1:1989//01//08:1989//12//31::%EC";/ "+:2:1927//01//01:1989//01//07::%EC%Ey";/ "+:1:1926//12//25:1926//12//31::%EC";/ commit dcd2b97dd1d695445d45beb4daa815cfe06691dd Author: Carlos O'Donell Date: Mon Apr 15 20:49:32 2019 +0200 malloc: Set and reset all hooks for tracing (Bug 16573) If an error occurs during the tracing operation, particularly during a call to lock_and_info() which calls _dl_addr, we may end up calling back into the malloc-subsystem and relock the loader lock and deadlock. For all intents and purposes the call to _dl_addr can call any of the malloc family API functions and so we should disable all tracing before calling such loader functions. This is similar to the strategy that the new malloc tracer takes when calling the real malloc, namely that all tracing ceases at the boundary to the real function and any faults at that point are the purvue of the library (though the new tracer does this on a per-thread basis in an MT-safe fashion). Since the new tracer and the hook deprecation are not yet complete we must fix these issues where we can. Tested on x86_64 with no regressions. Co-authored-by: Kwok Cheung Yeung Reviewed-by: DJ Delorie (cherry picked from commit e621246ec6393ea08ae50310f9d5e72500f8c9bc) diff --git a/ChangeLog b/ChangeLog index 3b5d24cf67..077d0dae29 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2019-04-09 Carlos O'Donell + Kwok Cheung Yeung + + [BZ #16573] + * malloc/mtrace.c: Define prototypes for all hooks. + (set_default_hooks): New function. + (set_trace_hooks): Likewise. + (save_default_hooks): Likewise. + (tr_freehook): Use new s*_hooks functions. + (tr_mallochook): Likewise. + (tr_reallochook): Likewise. + (tr_memalignhook): Likewise. + (mtrace): Likewise. + (muntrace): Likewise. + 2019-04-02 TAMUKI Shoichi [BZ #22964] diff --git a/NEWS b/NEWS index 703864ac75..117646df7b 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,7 @@ Major new features: The following bugs are resolved with this release: + [16573] malloc: Set and reset all hooks for tracing [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 diff --git a/malloc/mtrace.c b/malloc/mtrace.c index a2facf65ea..2fda262508 100644 --- a/malloc/mtrace.c +++ b/malloc/mtrace.c @@ -121,6 +121,41 @@ lock_and_info (const void *caller, Dl_info *mem) return res; } +static void tr_freehook (void *, const void *); +static void * tr_mallochook (size_t, const void *); +static void * tr_reallochook (void *, size_t, const void *); +static void * tr_memalignhook (size_t, size_t, const void *); + +/* Set all the default non-trace hooks. */ +static __always_inline void +set_default_hooks (void) +{ + __free_hook = tr_old_free_hook; + __malloc_hook = tr_old_malloc_hook; + __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; +} + +/* Set all of the tracing hooks used for mtrace. */ +static __always_inline void +set_trace_hooks (void) +{ + __free_hook = tr_freehook; + __malloc_hook = tr_mallochook; + __realloc_hook = tr_reallochook; + __memalign_hook = tr_memalignhook; +} + +/* Save the current set of hooks as the default hooks. */ +static __always_inline void +save_default_hooks (void) +{ + tr_old_free_hook = __free_hook; + tr_old_malloc_hook = __malloc_hook; + tr_old_realloc_hook = __realloc_hook; + tr_old_memalign_hook = __memalign_hook; +} + static void tr_freehook (void *ptr, const void *caller) { @@ -138,12 +173,12 @@ tr_freehook (void *ptr, const void *caller) tr_break (); __libc_lock_lock (lock); } - __free_hook = tr_old_free_hook; + set_default_hooks (); if (tr_old_free_hook != NULL) (*tr_old_free_hook)(ptr, caller); else free (ptr); - __free_hook = tr_freehook; + set_trace_hooks (); __libc_lock_unlock (lock); } @@ -155,12 +190,12 @@ tr_mallochook (size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_malloc_hook != NULL) hdr = (void *) (*tr_old_malloc_hook)(size, caller); else hdr = (void *) malloc (size); - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -185,16 +220,12 @@ tr_reallochook (void *ptr, size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __free_hook = tr_old_free_hook; - __malloc_hook = tr_old_malloc_hook; - __realloc_hook = tr_old_realloc_hook; + set_default_hooks (); if (tr_old_realloc_hook != NULL) hdr = (void *) (*tr_old_realloc_hook)(ptr, size, caller); else hdr = (void *) realloc (ptr, size); - __free_hook = tr_freehook; - __malloc_hook = tr_mallochook; - __realloc_hook = tr_reallochook; + set_trace_hooks (); tr_where (caller, info); if (hdr == NULL) @@ -230,14 +261,12 @@ tr_memalignhook (size_t alignment, size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __memalign_hook = tr_old_memalign_hook; - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_memalign_hook != NULL) hdr = (void *) (*tr_old_memalign_hook)(alignment, size, caller); else hdr = (void *) memalign (alignment, size); - __memalign_hook = tr_memalignhook; - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -305,14 +334,8 @@ mtrace (void) malloc_trace_buffer = mtb; setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE); fprintf (mallstream, "= Start\n"); - tr_old_free_hook = __free_hook; - __free_hook = tr_freehook; - tr_old_malloc_hook = __malloc_hook; - __malloc_hook = tr_mallochook; - tr_old_realloc_hook = __realloc_hook; - __realloc_hook = tr_reallochook; - tr_old_memalign_hook = __memalign_hook; - __memalign_hook = tr_memalignhook; + save_default_hooks (); + set_trace_hooks (); #ifdef _LIBC if (!added_atexit_handler) { @@ -338,10 +361,7 @@ muntrace (void) file. */ FILE *f = mallstream; mallstream = NULL; - __free_hook = tr_old_free_hook; - __malloc_hook = tr_old_malloc_hook; - __realloc_hook = tr_old_realloc_hook; - __memalign_hook = tr_old_memalign_hook; + set_default_hooks (); fprintf (f, "= End\n"); fclose (f); commit 42dfc13abf6fbb4c7a0215238eb636b7d374e0e0 Author: Mike Frysinger Date: Wed Apr 24 19:07:46 2019 +0200 memusagestat: use local glibc when linking [BZ #18465] The memusagestat is the only binary that has its own link line which causes it to be linked against the existing installed C library. It has been this way since it was originally committed in 1999, but I don't see any reason as to why. Since we want all the programs we build locally to be against the new copy of glibc, change the build to be like all other programs. (cherry picked from commit f9b645b4b0a10c43753296ce3fa40053fa44606a) diff --git a/ChangeLog b/ChangeLog index 077d0dae29..5291a88f85 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2019-04-24 Mike Frysinger + + [BZ #18465] + * malloc/Makefile (others): Add memusagestat. + ($(objpfx)memusagestat): Delete rule. + (LDLIBS-memusagestat): New variable. + 2019-04-09 Carlos O'Donell Kwok Cheung Yeung diff --git a/NEWS b/NEWS index 117646df7b..07e099b5ec 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,7 @@ Major new features: The following bugs are resolved with this release: [16573] malloc: Set and reset all hooks for tracing + [18465] memusagestat: use local glibc when linking [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 diff --git a/malloc/Makefile b/malloc/Makefile index ab2eed09c6..aadf602dfd 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -131,6 +131,7 @@ ifneq ($(cross-compiling),yes) # If the gd library is available we build the `memusagestat' program. ifneq ($(LIBGD),no) others: $(objpfx)memusage +others += memusagestat install-bin = memusagestat install-bin-script += memusage generated += memusagestat memusage @@ -154,8 +155,7 @@ cpp-srcs-left := $(memusagestat-modules) lib := memusagestat include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) -$(objpfx)memusagestat: $(memusagestat-modules:%=$(objpfx)%.o) - $(LINK.o) -o $@ $^ $(libgd-LDFLAGS) -lgd -lpng -lz -lm +LDLIBS-memusagestat = $(libgd-LDFLAGS) -lgd -lpng -lz -lm ifeq ($(run-built-tests),yes) ifeq (yes,$(build-shared)) commit 0744a268bc73e42b14b83e4cf3d083c6df6344e8 Author: Florian Weimer Date: Thu Apr 25 14:58:13 2019 +0200 Revert "memusagestat: use local glibc when linking [BZ #18465]" This reverts commit 42dfc13abf6fbb4c7a0215238eb636b7d374e0e0. The position of the -Wl,-rpath-link= options on the linker command line is not correct, so the new way of linking memusagestat does not always work. diff --git a/ChangeLog b/ChangeLog index 5291a88f85..077d0dae29 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,10 +1,3 @@ -2019-04-24 Mike Frysinger - - [BZ #18465] - * malloc/Makefile (others): Add memusagestat. - ($(objpfx)memusagestat): Delete rule. - (LDLIBS-memusagestat): New variable. - 2019-04-09 Carlos O'Donell Kwok Cheung Yeung diff --git a/NEWS b/NEWS index 07e099b5ec..117646df7b 100644 --- a/NEWS +++ b/NEWS @@ -14,7 +14,6 @@ Major new features: The following bugs are resolved with this release: [16573] malloc: Set and reset all hooks for tracing - [18465] memusagestat: use local glibc when linking [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 diff --git a/malloc/Makefile b/malloc/Makefile index aadf602dfd..ab2eed09c6 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -131,7 +131,6 @@ ifneq ($(cross-compiling),yes) # If the gd library is available we build the `memusagestat' program. ifneq ($(LIBGD),no) others: $(objpfx)memusage -others += memusagestat install-bin = memusagestat install-bin-script += memusage generated += memusagestat memusage @@ -155,7 +154,8 @@ cpp-srcs-left := $(memusagestat-modules) lib := memusagestat include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left)) -LDLIBS-memusagestat = $(libgd-LDFLAGS) -lgd -lpng -lz -lm +$(objpfx)memusagestat: $(memusagestat-modules:%=$(objpfx)%.o) + $(LINK.o) -o $@ $^ $(libgd-LDFLAGS) -lgd -lpng -lz -lm ifeq ($(run-built-tests),yes) ifeq (yes,$(build-shared)) commit f62d21a1f0107e6f7182f346293583c9121a877d Author: Adhemerval Zanella Date: Fri Apr 12 17:39:53 2019 -0300 support: Add support_capture_subprogram Its API is similar to support_capture_subprocess, but rather creates a new process based on the input path and arguments. Under the hoods it uses posix_spawn to create the new process. It also allows the use of other support_capture_* functions to check for expected results and free the resources. Checked on x86_64-linux-gnu. * support/Makefile (libsupport-routines): Add support_subprocess, xposix_spawn, xposix_spawn_file_actions_addclose, and xposix_spawn_file_actions_adddup2. (tst-support_capture_subprocess-ARGS): New rule. * support/capture_subprocess.h (support_capture_subprogram): New prototype. * support/support_capture_subprocess.c (support_capture_subprocess): Refactor to use support_subprocess and support_capture_poll. (support_capture_subprogram): New function. * support/tst-support_capture_subprocess.c (write_mode_to_str, str_to_write_mode, test_common, parse_int, handle_restart, do_subprocess, do_subprogram, do_multiple_tests): New functions. (do_test): Add support_capture_subprogram tests. * support/subprocess.h: New file. * support/support_subprocess.c: Likewise. * support/xposix_spawn.c: Likewise. * support/xposix_spawn_file_actions_addclose.c: Likewise. * support/xposix_spawn_file_actions_adddup2.c: Likewise. * support/xspawn.h: Likewise. Reviewed-by: Carlos O'Donell (cherry picked from commit 0e169691290a6d2187a4ff41495fc5678cbfdcdc) diff --git a/ChangeLog b/ChangeLog index 077d0dae29..2524e25697 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2019-04-17 Adhemerval Zanella + + * support/Makefile (libsupport-routines): Add support_subprocess, + xposix_spawn, xposix_spawn_file_actions_addclose, and + xposix_spawn_file_actions_adddup2. + (tst-support_capture_subprocess-ARGS): New rule. + * support/capture_subprocess.h (support_capture_subprogram): New + prototype. + * support/support_capture_subprocess.c (support_capture_subprocess): + Refactor to use support_subprocess and support_capture_poll. + (support_capture_subprogram): New function. + * support/tst-support_capture_subprocess.c (write_mode_to_str, + str_to_write_mode, test_common, parse_int, handle_restart, + do_subprocess, do_subprogram, do_multiple_tests): New functions. + (do_test): Add support_capture_subprogram tests. + * support/subprocess.h: New file. + * support/support_subprocess.c: Likewise. + * support/xposix_spawn.c: Likewise. + * support/xposix_spawn_file_actions_addclose.c: Likewise. + * support/xposix_spawn_file_actions_adddup2.c: Likewise. + * support/xspawn.h: Likewise. + 2019-04-09 Carlos O'Donell Kwok Cheung Yeung diff --git a/support/Makefile b/support/Makefile index c15b93647c..8d61de6c57 100644 --- a/support/Makefile +++ b/support/Makefile @@ -63,6 +63,7 @@ libsupport-routines = \ support_record_failure \ support_run_diff \ support_shared_allocate \ + support_subprocess \ support_test_compare_blob \ support_test_compare_failure \ support_test_compare_string \ @@ -148,6 +149,9 @@ libsupport-routines = \ xsignal \ xsigstack \ xsocket \ + xposix_spawn \ + xposix_spawn_file_actions_addclose \ + xposix_spawn_file_actions_adddup2 \ xstrdup \ xstrndup \ xsymlink \ @@ -223,4 +227,6 @@ endif $(objpfx)tst-support_format_dns_packet: $(common-objpfx)resolv/libresolv.so +tst-support_capture_subprocess-ARGS = -- $(host-test-program-cmd) + include ../Rules diff --git a/support/capture_subprocess.h b/support/capture_subprocess.h index 2dce42e3a3..2832cfc635 100644 --- a/support/capture_subprocess.h +++ b/support/capture_subprocess.h @@ -35,6 +35,12 @@ struct support_capture_subprocess struct support_capture_subprocess support_capture_subprocess (void (*callback) (void *), void *closure); +/* Issue FILE with ARGV arguments by using posix_spawn and capture standard + output, standard error, and the exit status. The out.buffer and err.buffer + are handle as support_capture_subprocess. */ +struct support_capture_subprocess support_capture_subprogram + (const char *file, char *const argv[]); + /* Deallocate the subprocess data captured by support_capture_subprocess. */ void support_capture_subprocess_free (struct support_capture_subprocess *); diff --git a/support/subprocess.h b/support/subprocess.h new file mode 100644 index 0000000000..c031878d94 --- /dev/null +++ b/support/subprocess.h @@ -0,0 +1,49 @@ +/* Create a subprocess. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef SUPPORT_SUBPROCESS_H +#define SUPPORT_SUBPROCESS_H + +#include + +struct support_subprocess +{ + int stdout_pipe[2]; + int stderr_pipe[2]; + pid_t pid; +}; + +/* Invoke CALLBACK (CLOSURE) in a subprocess created with fork and return + its PID, a pipe redirected to STDOUT, and a pipe redirected to STDERR. */ +struct support_subprocess support_subprocess + (void (*callback) (void *), void *closure); + +/* Issue FILE with ARGV arguments by using posix_spawn and return is PID, a + pipe redirected to STDOUT, and a pipe redirected to STDERR. */ +struct support_subprocess support_subprogram + (const char *file, char *const argv[]); + +/* Wait for the subprocess indicated by PROC::PID. Return the status + indicate by waitpid call. */ +int support_process_wait (struct support_subprocess *proc); + +/* Terminate the subprocess indicated by PROC::PID, first with a SIGTERM and + then with a SIGKILL. Return the status as for waitpid call. */ +int support_process_terminate (struct support_subprocess *proc); + +#endif diff --git a/support/support_capture_subprocess.c b/support/support_capture_subprocess.c index 167514faf1..948ce5a0c6 100644 --- a/support/support_capture_subprocess.c +++ b/support/support_capture_subprocess.c @@ -16,6 +16,7 @@ License along with the GNU C Library; if not, see . */ +#include #include #include @@ -23,6 +24,7 @@ #include #include #include +#include static void transfer (const char *what, struct pollfd *pfd, struct xmemstream *stream) @@ -50,59 +52,53 @@ transfer (const char *what, struct pollfd *pfd, struct xmemstream *stream) } } -struct support_capture_subprocess -support_capture_subprocess (void (*callback) (void *), void *closure) +static void +support_capture_poll (struct support_capture_subprocess *result, + struct support_subprocess *proc) { - struct support_capture_subprocess result; - xopen_memstream (&result.out); - xopen_memstream (&result.err); - - int stdout_pipe[2]; - xpipe (stdout_pipe); - TEST_VERIFY (stdout_pipe[0] > STDERR_FILENO); - TEST_VERIFY (stdout_pipe[1] > STDERR_FILENO); - int stderr_pipe[2]; - xpipe (stderr_pipe); - TEST_VERIFY (stderr_pipe[0] > STDERR_FILENO); - TEST_VERIFY (stderr_pipe[1] > STDERR_FILENO); - - TEST_VERIFY (fflush (stdout) == 0); - TEST_VERIFY (fflush (stderr) == 0); - - pid_t pid = xfork (); - if (pid == 0) - { - xclose (stdout_pipe[0]); - xclose (stderr_pipe[0]); - xdup2 (stdout_pipe[1], STDOUT_FILENO); - xdup2 (stderr_pipe[1], STDERR_FILENO); - xclose (stdout_pipe[1]); - xclose (stderr_pipe[1]); - callback (closure); - _exit (0); - } - xclose (stdout_pipe[1]); - xclose (stderr_pipe[1]); - struct pollfd fds[2] = { - { .fd = stdout_pipe[0], .events = POLLIN }, - { .fd = stderr_pipe[0], .events = POLLIN }, + { .fd = proc->stdout_pipe[0], .events = POLLIN }, + { .fd = proc->stderr_pipe[0], .events = POLLIN }, }; do { xpoll (fds, 2, -1); - transfer ("stdout", &fds[0], &result.out); - transfer ("stderr", &fds[1], &result.err); + transfer ("stdout", &fds[0], &result->out); + transfer ("stderr", &fds[1], &result->err); } while (fds[0].events != 0 || fds[1].events != 0); - xclose (stdout_pipe[0]); - xclose (stderr_pipe[0]); - xfclose_memstream (&result.out); - xfclose_memstream (&result.err); - xwaitpid (pid, &result.status, 0); + xfclose_memstream (&result->out); + xfclose_memstream (&result->err); + + result->status = support_process_wait (proc); +} + +struct support_capture_subprocess +support_capture_subprocess (void (*callback) (void *), void *closure) +{ + struct support_capture_subprocess result; + xopen_memstream (&result.out); + xopen_memstream (&result.err); + + struct support_subprocess proc = support_subprocess (callback, closure); + + support_capture_poll (&result, &proc); + return result; +} + +struct support_capture_subprocess +support_capture_subprogram (const char *file, char *const argv[]) +{ + struct support_capture_subprocess result; + xopen_memstream (&result.out); + xopen_memstream (&result.err); + + struct support_subprocess proc = support_subprogram (file, argv); + + support_capture_poll (&result, &proc); return result; } diff --git a/support/support_subprocess.c b/support/support_subprocess.c new file mode 100644 index 0000000000..0c8cc6af30 --- /dev/null +++ b/support/support_subprocess.c @@ -0,0 +1,152 @@ +/* Create subprocess. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static struct support_subprocess +support_suprocess_init (void) +{ + struct support_subprocess result; + + xpipe (result.stdout_pipe); + TEST_VERIFY (result.stdout_pipe[0] > STDERR_FILENO); + TEST_VERIFY (result.stdout_pipe[1] > STDERR_FILENO); + + xpipe (result.stderr_pipe); + TEST_VERIFY (result.stderr_pipe[0] > STDERR_FILENO); + TEST_VERIFY (result.stderr_pipe[1] > STDERR_FILENO); + + TEST_VERIFY (fflush (stdout) == 0); + TEST_VERIFY (fflush (stderr) == 0); + + return result; +} + +struct support_subprocess +support_subprocess (void (*callback) (void *), void *closure) +{ + struct support_subprocess result = support_suprocess_init (); + + result.pid = xfork (); + if (result.pid == 0) + { + xclose (result.stdout_pipe[0]); + xclose (result.stderr_pipe[0]); + xdup2 (result.stdout_pipe[1], STDOUT_FILENO); + xdup2 (result.stderr_pipe[1], STDERR_FILENO); + xclose (result.stdout_pipe[1]); + xclose (result.stderr_pipe[1]); + callback (closure); + _exit (0); + } + xclose (result.stdout_pipe[1]); + xclose (result.stderr_pipe[1]); + + return result; +} + +struct support_subprocess +support_subprogram (const char *file, char *const argv[]) +{ + struct support_subprocess result = support_suprocess_init (); + + posix_spawn_file_actions_t fa; + /* posix_spawn_file_actions_init does not fail. */ + posix_spawn_file_actions_init (&fa); + + xposix_spawn_file_actions_addclose (&fa, result.stdout_pipe[0]); + xposix_spawn_file_actions_addclose (&fa, result.stderr_pipe[0]); + xposix_spawn_file_actions_adddup2 (&fa, result.stdout_pipe[1], STDOUT_FILENO); + xposix_spawn_file_actions_adddup2 (&fa, result.stderr_pipe[1], STDERR_FILENO); + xposix_spawn_file_actions_addclose (&fa, result.stdout_pipe[1]); + xposix_spawn_file_actions_addclose (&fa, result.stderr_pipe[1]); + + result.pid = xposix_spawn (file, &fa, NULL, argv, NULL); + + xclose (result.stdout_pipe[1]); + xclose (result.stderr_pipe[1]); + + return result; +} + +int +support_process_wait (struct support_subprocess *proc) +{ + xclose (proc->stdout_pipe[0]); + xclose (proc->stderr_pipe[0]); + + int status; + xwaitpid (proc->pid, &status, 0); + return status; +} + + +static bool +support_process_kill (int pid, int signo, int *status) +{ + /* Kill the whole process group. */ + kill (-pid, signo); + /* In case setpgid failed in the child, kill it individually too. */ + kill (pid, signo); + + /* Wait for it to terminate. */ + pid_t killed; + for (int i = 0; i < 5; ++i) + { + int status; + killed = xwaitpid (pid, &status, WNOHANG|WUNTRACED); + if (killed != 0) + break; + + /* Delay, give the system time to process the kill. If the + nanosleep() call return prematurely, all the better. We + won't restart it since this probably means the child process + finally died. */ + nanosleep (&((struct timespec) { 0, 100000000 }), NULL); + } + if (killed != 0 && killed != pid) + return false; + + return true; +} + +int +support_process_terminate (struct support_subprocess *proc) +{ + xclose (proc->stdout_pipe[0]); + xclose (proc->stderr_pipe[0]); + + int status; + pid_t killed = xwaitpid (proc->pid, &status, WNOHANG|WUNTRACED); + if (killed != 0 && killed == proc->pid) + return status; + + /* Subprocess is still running, terminate it. */ + if (!support_process_kill (proc->pid, SIGTERM, &status) ) + support_process_kill (proc->pid, SIGKILL, &status); + + return status; +} diff --git a/support/tst-support_capture_subprocess.c b/support/tst-support_capture_subprocess.c index d8ba42ea8b..ab363e41ac 100644 --- a/support/tst-support_capture_subprocess.c +++ b/support/tst-support_capture_subprocess.c @@ -23,8 +23,20 @@ #include #include #include +#include #include #include +#include +#include +#include +#include +#include + +/* Nonzero if the program gets called via 'exec'. */ +static int restart; + +/* Hold the four initial argument used to respawn the process. */ +static char *initial_argv[5]; /* Write one byte at *P to FD and advance *P. Do nothing if *P is '\0'. */ @@ -42,6 +54,30 @@ transfer (const unsigned char **p, int fd) enum write_mode { out_first, err_first, interleave, write_mode_last = interleave }; +static const char * +write_mode_to_str (enum write_mode mode) +{ + switch (mode) + { + case out_first: return "out_first"; + case err_first: return "err_first"; + case interleave: return "interleave"; + default: return "write_mode_last"; + } +} + +static enum write_mode +str_to_write_mode (const char *mode) +{ + if (strcmp (mode, "out_first") == 0) + return out_first; + else if (strcmp (mode, "err_first") == 0) + return err_first; + else if (strcmp (mode, "interleave") == 0) + return interleave; + return write_mode_last; +} + /* Describe what to write in the subprocess. */ struct test { @@ -52,11 +88,9 @@ struct test int status; }; -/* For use with support_capture_subprocess. */ -static void -callback (void *closure) +_Noreturn static void +test_common (const struct test *test) { - const struct test *test = closure; bool mode_ok = false; switch (test->write_mode) { @@ -95,6 +129,40 @@ callback (void *closure) exit (test->status); } +static int +parse_int (const char *str) +{ + char *endptr; + long int ret = strtol (str, &endptr, 10); + TEST_COMPARE (errno, 0); + TEST_VERIFY (ret >= 0 && ret <= INT_MAX); + return ret; +} + +/* For use with support_capture_subprogram. */ +_Noreturn static void +handle_restart (char *out, char *err, const char *write_mode, + const char *signal, const char *status) +{ + struct test test = + { + out, + err, + str_to_write_mode (write_mode), + parse_int (signal), + parse_int (status) + }; + test_common (&test); +} + +/* For use with support_capture_subprocess. */ +_Noreturn static void +callback (void *closure) +{ + const struct test *test = closure; + test_common (test); +} + /* Create a heap-allocated random string of letters. */ static char * random_string (size_t length) @@ -130,12 +198,59 @@ check_stream (const char *what, const struct xmemstream *stream, } } +static struct support_capture_subprocess +do_subprocess (struct test *test) +{ + return support_capture_subprocess (callback, test); +} + +static struct support_capture_subprocess +do_subprogram (const struct test *test) +{ + /* Three digits per byte plus null terminator. */ + char signalstr[3 * sizeof(int) + 1]; + snprintf (signalstr, sizeof (signalstr), "%d", test->signal); + char statusstr[3 * sizeof(int) + 1]; + snprintf (statusstr, sizeof (statusstr), "%d", test->status); + + int argc = 0; + enum { + /* 4 elements from initial_argv (path to ld.so, '--library-path', the + path', and application name'), 2 for restart argument ('--direct', + '--restart'), 5 arguments plus NULL. */ + argv_size = 12 + }; + char *args[argv_size]; + + for (char **arg = initial_argv; *arg != NULL; arg++) + args[argc++] = *arg; + + args[argc++] = (char*) "--direct"; + args[argc++] = (char*) "--restart"; + + args[argc++] = test->out; + args[argc++] = test->err; + args[argc++] = (char*) write_mode_to_str (test->write_mode); + args[argc++] = signalstr; + args[argc++] = statusstr; + args[argc] = NULL; + TEST_VERIFY (argc < argv_size); + + return support_capture_subprogram (args[0], args); +} + +enum test_type +{ + subprocess, + subprogram, +}; + static int -do_test (void) +do_multiple_tests (enum test_type type) { const int lengths[] = {0, 1, 17, 512, 20000, -1}; - /* Test multiple combinations of support_capture_subprocess. + /* Test multiple combinations of support_capture_sub{process,program}. length_idx_stdout: Index into the lengths array above, controls how many bytes are written by the subprocess to @@ -164,8 +279,10 @@ do_test (void) TEST_VERIFY (strlen (test.out) == lengths[length_idx_stdout]); TEST_VERIFY (strlen (test.err) == lengths[length_idx_stderr]); - struct support_capture_subprocess result - = support_capture_subprocess (callback, &test); + struct support_capture_subprocess result + = type == subprocess ? do_subprocess (&test) + : do_subprogram (&test); + check_stream ("stdout", &result.out, test.out); check_stream ("stderr", &result.err, test.err); @@ -199,4 +316,54 @@ do_test (void) return 0; } +static int +do_test (int argc, char *argv[]) +{ + /* We must have either: + + - one or four parameters if called initially: + + argv[1]: path for ld.so optional + + argv[2]: "--library-path" optional + + argv[3]: the library path optional + + argv[4]: the application name + + - six parameters left if called through re-execution: + + argv[1]: the application name + + argv[2]: the stdout to print + + argv[3]: the stderr to print + + argv[4]: the write mode to use + + argv[5]: the signal to issue + + argv[6]: the exit status code to use + + * When built with --enable-hardcoded-path-in-tests or issued without + using the loader directly. + */ + + if (argc != (restart ? 6 : 5) && argc != (restart ? 6 : 2)) + FAIL_EXIT1 ("wrong number of arguments (%d)", argc); + + if (restart) + { + handle_restart (argv[1], /* stdout */ + argv[2], /* stderr */ + argv[3], /* write_mode */ + argv[4], /* signal */ + argv[5]); /* status */ + } + + initial_argv[0] = argv[1]; /* path for ld.so */ + initial_argv[1] = argv[2]; /* "--library-path" */ + initial_argv[2] = argv[3]; /* the library path */ + initial_argv[3] = argv[4]; /* the application name */ + initial_argv[4] = NULL; + + do_multiple_tests (subprocess); + do_multiple_tests (subprogram); + + return 0; +} + +#define CMDLINE_OPTIONS \ + { "restart", no_argument, &restart, 1 }, +#define TEST_FUNCTION_ARGV do_test #include diff --git a/support/xposix_spawn.c b/support/xposix_spawn.c new file mode 100644 index 0000000000..e846017632 --- /dev/null +++ b/support/xposix_spawn.c @@ -0,0 +1,32 @@ +/* xposix_spawn implementation. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include + +pid_t +xposix_spawn (const char *file, const posix_spawn_file_actions_t *fa, + const posix_spawnattr_t *attr, char *const args[], + char *const envp[]) +{ + pid_t pid; + int status = posix_spawn (&pid, file, fa, attr, args, envp); + if (status != 0) + FAIL_EXIT1 ("posix_spawn to %s file failed: %m", file); + return pid; +} diff --git a/support/xposix_spawn_file_actions_addclose.c b/support/xposix_spawn_file_actions_addclose.c new file mode 100644 index 0000000000..eed54a6514 --- /dev/null +++ b/support/xposix_spawn_file_actions_addclose.c @@ -0,0 +1,29 @@ +/* xposix_spawn_file_actions_addclose implementation. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include + +int +xposix_spawn_file_actions_addclose (posix_spawn_file_actions_t *fa, int fd) +{ + int status = posix_spawn_file_actions_addclose (fa, fd); + if (status == -1) + FAIL_EXIT1 ("posix_spawn_file_actions_addclose failed: %m\n"); + return status; +} diff --git a/support/xposix_spawn_file_actions_adddup2.c b/support/xposix_spawn_file_actions_adddup2.c new file mode 100644 index 0000000000..a43b6490be --- /dev/null +++ b/support/xposix_spawn_file_actions_adddup2.c @@ -0,0 +1,30 @@ +/* xposix_spawn_file_actions_adddup2 implementation. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include + +int +xposix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *fa, int fd, + int newfd) +{ + int status = posix_spawn_file_actions_adddup2 (fa, fd, newfd); + if (status == -1) + FAIL_EXIT1 ("posix_spawn_file_actions_adddup2 failed: %m\n"); + return status; +} diff --git a/support/xspawn.h b/support/xspawn.h new file mode 100644 index 0000000000..bbf89132e4 --- /dev/null +++ b/support/xspawn.h @@ -0,0 +1,34 @@ +/* posix_spawn with support checks. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef SUPPORT_XSPAWN_H +#define SUPPORT_XSPAWN_H + +#include + +__BEGIN_DECLS + +int xposix_spawn_file_actions_addclose (posix_spawn_file_actions_t *, int); +int xposix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *, int, int); + +pid_t xposix_spawn (const char *, const posix_spawn_file_actions_t *, + const posix_spawnattr_t *, char *const [], char *const []); + +__END_DECLS + +#endif commit eaea1dfbe95a31c29adc259100569962cddb6f19 Author: Adhemerval Zanella Date: Fri Apr 26 13:58:31 2019 +0200 elf: Fix pldd (BZ#18035) Since 9182aa67994 (Fix vDSO l_name for GDB's, BZ#387) the initial link_map for executable itself and loader will have both l_name and l_libname->name holding the same value due: elf/dl-object.c 95 new->l_name = *realname ? realname : (char *) newname->name + libname_len - 1; Since newname->name points to new->l_libname->name. This leads to pldd to an infinite call at: elf/pldd-xx.c 203 again: 204 while (1) 205 { 206 ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset); 228 /* Try the l_libname element. */ 229 struct E(libname_list) ln; 230 if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln)) 231 { 232 name_offset = ln.name; 233 goto again; 234 } Since the value at ln.name (l_libname->name) will be the same as previously read. The straightforward fix is just avoid the check and read the new list entry. I checked also against binaries issues with old loaders with fix for BZ#387, and pldd could dump the shared objects. Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, and powerpc64le-linux-gnu. [BZ #18035] * elf/Makefile (tests-container): Add tst-pldd. * elf/pldd-xx.c: Use _Static_assert in of pldd_assert. (E(find_maps)): Avoid use alloca, use default read file operations instead of explicit LFS names, and fix infinite loop. * elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers. (get_process_info): Use _Static_assert instead of assert, use default directory operations instead of explicit LFS names, and free some leadek pointers. * elf/tst-pldd.c: New file. (cherry picked from commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f) diff --git a/ChangeLog b/ChangeLog index 2524e25697..5af8e27ab9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2019-04-23 Adhemerval Zanella + + [BZ #18035] + * elf/Makefile (tests-container): Add tst-pldd. + * elf/pldd-xx.c: Use _Static_assert in of pldd_assert. + (E(find_maps)): Avoid use alloca, use default read file operations + instead of explicit LFS names, and fix infinite loop. + * elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers. + (get_process_info): Use _Static_assert instead of assert, use default + directory operations instead of explicit LFS names, and free some + leadek pointers. + * elf/tst-pldd.c: New file. + 2019-04-17 Adhemerval Zanella * support/Makefile (libsupport-routines): Add support_subprocess, diff --git a/NEWS b/NEWS index 117646df7b..b39a0ccf91 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,7 @@ Major new features: The following bugs are resolved with this release: [16573] malloc: Set and reset all hooks for tracing + [18035] Fix pldd hang [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 diff --git a/elf/Makefile b/elf/Makefile index 9cf5cd8dfd..e7457e809f 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -194,6 +194,7 @@ tests-internal += loadtest unload unload2 circleload1 \ tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \ tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \ tst-create_format1 +tests-container += tst-pldd ifeq ($(build-hardcoded-path-in-tests),yes) tests += tst-dlopen-aout tst-dlopen-aout-no-pie = yes diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c index 547f840ee1..756f6d7a1c 100644 --- a/elf/pldd-xx.c +++ b/elf/pldd-xx.c @@ -23,10 +23,6 @@ #define EW_(e, w, t) EW__(e, w, _##t) #define EW__(e, w, t) e##w##t -#define pldd_assert(name, exp) \ - typedef int __assert_##name[((exp) != 0) - 1] - - struct E(link_map) { EW(Addr) l_addr; @@ -39,12 +35,12 @@ struct E(link_map) EW(Addr) l_libname; }; #if CLASS == __ELF_NATIVE_CLASS -pldd_assert (l_addr, (offsetof (struct link_map, l_addr) - == offsetof (struct E(link_map), l_addr))); -pldd_assert (l_name, (offsetof (struct link_map, l_name) - == offsetof (struct E(link_map), l_name))); -pldd_assert (l_next, (offsetof (struct link_map, l_next) - == offsetof (struct E(link_map), l_next))); +_Static_assert (offsetof (struct link_map, l_addr) + == offsetof (struct E(link_map), l_addr), "l_addr"); +_Static_assert (offsetof (struct link_map, l_name) + == offsetof (struct E(link_map), l_name), "l_name"); +_Static_assert (offsetof (struct link_map, l_next) + == offsetof (struct E(link_map), l_next), "l_next"); #endif @@ -54,10 +50,10 @@ struct E(libname_list) EW(Addr) next; }; #if CLASS == __ELF_NATIVE_CLASS -pldd_assert (name, (offsetof (struct libname_list, name) - == offsetof (struct E(libname_list), name))); -pldd_assert (next, (offsetof (struct libname_list, next) - == offsetof (struct E(libname_list), next))); +_Static_assert (offsetof (struct libname_list, name) + == offsetof (struct E(libname_list), name), "name"); +_Static_assert (offsetof (struct libname_list, next) + == offsetof (struct E(libname_list), next), "next"); #endif struct E(r_debug) @@ -69,16 +65,17 @@ struct E(r_debug) EW(Addr) r_map; }; #if CLASS == __ELF_NATIVE_CLASS -pldd_assert (r_version, (offsetof (struct r_debug, r_version) - == offsetof (struct E(r_debug), r_version))); -pldd_assert (r_map, (offsetof (struct r_debug, r_map) - == offsetof (struct E(r_debug), r_map))); +_Static_assert (offsetof (struct r_debug, r_version) + == offsetof (struct E(r_debug), r_version), "r_version"); +_Static_assert (offsetof (struct r_debug, r_map) + == offsetof (struct E(r_debug), r_map), "r_map"); #endif static int -E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) +E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv, + size_t auxv_size) { EW(Addr) phdr = 0; unsigned int phnum = 0; @@ -104,12 +101,9 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) if (phdr == 0 || phnum == 0 || phent == 0) error (EXIT_FAILURE, 0, gettext ("cannot find program header of process")); - EW(Phdr) *p = alloca (phnum * phent); - if (pread64 (memfd, p, phnum * phent, phdr) != phnum * phent) - { - error (0, 0, gettext ("cannot read program header")); - return EXIT_FAILURE; - } + EW(Phdr) *p = xmalloc (phnum * phent); + if (pread (memfd, p, phnum * phent, phdr) != phnum * phent) + error (EXIT_FAILURE, 0, gettext ("cannot read program header")); /* Determine the load offset. We need this for interpreting the other program header entries so we do this in a separate loop. @@ -129,24 +123,18 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) if (p[i].p_type == PT_DYNAMIC) { EW(Dyn) *dyn = xmalloc (p[i].p_filesz); - if (pread64 (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr) + if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr) != p[i].p_filesz) - { - error (0, 0, gettext ("cannot read dynamic section")); - return EXIT_FAILURE; - } + error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section")); /* Search for the DT_DEBUG entry. */ for (unsigned int j = 0; j < p[i].p_filesz / sizeof (EW(Dyn)); ++j) if (dyn[j].d_tag == DT_DEBUG && dyn[j].d_un.d_ptr != 0) { struct E(r_debug) r; - if (pread64 (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr) + if (pread (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr) != sizeof (r)) - { - error (0, 0, gettext ("cannot read r_debug")); - return EXIT_FAILURE; - } + error (EXIT_FAILURE, 0, gettext ("cannot read r_debug")); if (r.r_map != 0) { @@ -160,13 +148,10 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) } else if (p[i].p_type == PT_INTERP) { - interp = alloca (p[i].p_filesz); - if (pread64 (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr) + interp = xmalloc (p[i].p_filesz); + if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr) != p[i].p_filesz) - { - error (0, 0, gettext ("cannot read program interpreter")); - return EXIT_FAILURE; - } + error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter")); } if (list == 0) @@ -174,14 +159,16 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) if (interp == NULL) { // XXX check whether the executable itself is the loader - return EXIT_FAILURE; + exit (EXIT_FAILURE); } // XXX perhaps try finding ld.so and _r_debug in it - - return EXIT_FAILURE; + exit (EXIT_FAILURE); } + free (p); + free (interp); + /* Print the PID and program name first. */ printf ("%lu:\t%s\n", (unsigned long int) pid, exe); @@ -192,47 +179,27 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) do { struct E(link_map) m; - if (pread64 (memfd, &m, sizeof (m), list) != sizeof (m)) - { - error (0, 0, gettext ("cannot read link map")); - status = EXIT_FAILURE; - goto out; - } + if (pread (memfd, &m, sizeof (m), list) != sizeof (m)) + error (EXIT_FAILURE, 0, gettext ("cannot read link map")); EW(Addr) name_offset = m.l_name; - again: while (1) { - ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset); + ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset); if (n == -1) - { - error (0, 0, gettext ("cannot read object name")); - status = EXIT_FAILURE; - goto out; - } + error (EXIT_FAILURE, 0, gettext ("cannot read object name")); if (memchr (tmpbuf.data, '\0', n) != NULL) break; if (!scratch_buffer_grow (&tmpbuf)) - { - error (0, 0, gettext ("cannot allocate buffer for object name")); - status = EXIT_FAILURE; - goto out; - } + error (EXIT_FAILURE, 0, + gettext ("cannot allocate buffer for object name")); } - if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name - && m.l_libname != 0) - { - /* Try the l_libname element. */ - struct E(libname_list) ln; - if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln)) - { - name_offset = ln.name; - goto again; - } - } + /* The m.l_name and m.l_libname.name for loader linkmap points to same + values (since BZ#387 fix). Trying to use l_libname name as the + shared object name might lead to an infinite loop (BZ#18035). */ /* Skip over the executable. */ if (((char *)tmpbuf.data)[0] != '\0') @@ -242,7 +209,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) } while (list != 0); - out: scratch_buffer_free (&tmpbuf); return status; } diff --git a/elf/pldd.c b/elf/pldd.c index f3fac4e487..69629bd5d2 100644 --- a/elf/pldd.c +++ b/elf/pldd.c @@ -17,23 +17,17 @@ License along with the GNU C Library; if not, see . */ -#include +#define _FILE_OFFSET_BITS 64 + #include -#include #include -#include -#include #include #include #include -#include -#include #include #include -#include #include #include -#include #include #include @@ -76,14 +70,9 @@ static struct argp argp = options, parse_opt, args_doc, doc, NULL, more_help, NULL }; -// File descriptor of /proc/*/mem file. -static int memfd; - -/* Name of the executable */ -static char *exe; /* Local functions. */ -static int get_process_info (int dfd, long int pid); +static int get_process_info (const char *exe, int dfd, long int pid); static void wait_for_ptrace_stop (long int pid); @@ -102,8 +91,10 @@ main (int argc, char *argv[]) return 1; } - assert (sizeof (pid_t) == sizeof (int) - || sizeof (pid_t) == sizeof (long int)); + _Static_assert (sizeof (pid_t) == sizeof (int) + || sizeof (pid_t) == sizeof (long int), + "sizeof (pid_t) != sizeof (int) or sizeof (long int)"); + char *endp; errno = 0; long int pid = strtol (argv[remaining], &endp, 10); @@ -119,25 +110,24 @@ main (int argc, char *argv[]) if (dfd == -1) error (EXIT_FAILURE, errno, gettext ("cannot open %s"), buf); - struct scratch_buffer exebuf; - scratch_buffer_init (&exebuf); + /* Name of the executable */ + struct scratch_buffer exe; + scratch_buffer_init (&exe); ssize_t nexe; while ((nexe = readlinkat (dfd, "exe", - exebuf.data, exebuf.length)) == exebuf.length) + exe.data, exe.length)) == exe.length) { - if (!scratch_buffer_grow (&exebuf)) + if (!scratch_buffer_grow (&exe)) { nexe = -1; break; } } if (nexe == -1) - exe = (char *) ""; + /* Default stack allocation is at least 1024. */ + snprintf (exe.data, exe.length, ""); else - { - exe = exebuf.data; - exe[nexe] = '\0'; - } + ((char*)exe.data)[nexe] = '\0'; /* Stop all threads since otherwise the list of loaded modules might change while we are reading it. */ @@ -155,8 +145,8 @@ main (int argc, char *argv[]) error (EXIT_FAILURE, errno, gettext ("cannot prepare reading %s/task"), buf); - struct dirent64 *d; - while ((d = readdir64 (dir)) != NULL) + struct dirent *d; + while ((d = readdir (dir)) != NULL) { if (! isdigit (d->d_name[0])) continue; @@ -182,7 +172,7 @@ main (int argc, char *argv[]) wait_for_ptrace_stop (tid); - struct thread_list *newp = alloca (sizeof (*newp)); + struct thread_list *newp = xmalloc (sizeof (*newp)); newp->tid = tid; newp->next = thread_list; thread_list = newp; @@ -190,17 +180,22 @@ main (int argc, char *argv[]) closedir (dir); - int status = get_process_info (dfd, pid); + if (thread_list == NULL) + error (EXIT_FAILURE, 0, gettext ("no valid %s/task entries"), buf); + + int status = get_process_info (exe.data, dfd, pid); - assert (thread_list != NULL); do { ptrace (PTRACE_DETACH, thread_list->tid, NULL, NULL); + struct thread_list *prev = thread_list; thread_list = thread_list->next; + free (prev); } while (thread_list != NULL); close (dfd); + scratch_buffer_free (&exe); return status; } @@ -281,9 +276,10 @@ warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\ static int -get_process_info (int dfd, long int pid) +get_process_info (const char *exe, int dfd, long int pid) { - memfd = openat (dfd, "mem", O_RDONLY); + /* File descriptor of /proc//mem file. */ + int memfd = openat (dfd, "mem", O_RDONLY); if (memfd == -1) goto no_info; @@ -333,9 +329,9 @@ get_process_info (int dfd, long int pid) int retval; if (e_ident[EI_CLASS] == ELFCLASS32) - retval = find_maps32 (pid, auxv, auxv_size); + retval = find_maps32 (exe, memfd, pid, auxv, auxv_size); else - retval = find_maps64 (pid, auxv, auxv_size); + retval = find_maps64 (exe, memfd, pid, auxv, auxv_size); free (auxv); close (memfd); diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c new file mode 100644 index 0000000000..ed19cedd05 --- /dev/null +++ b/elf/tst-pldd.c @@ -0,0 +1,118 @@ +/* Basic tests for pldd program. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +static void +target_process (void *arg) +{ + pause (); +} + +/* The test runs in a container because pldd does not support tracing + a binary started by the loader iself (as with testrun.sh). */ + +static int +do_test (void) +{ + /* Create a copy of current test to check with pldd. */ + struct support_subprocess target = support_subprocess (target_process, NULL); + + /* Run 'pldd' on test subprocess. */ + struct support_capture_subprocess pldd; + { + /* Three digits per byte plus null terminator. */ + char pid[3 * sizeof (uint32_t) + 1]; + snprintf (pid, array_length (pid), "%d", target.pid); + + const char prog[] = "/usr/bin/pldd"; + + pldd = support_capture_subprogram (prog, + (char *const []) { (char *) prog, pid, NULL }); + + support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); + } + + /* Check 'pldd' output. The test is expected to be linked against only + loader and libc. */ + { + pid_t pid; + char buffer[512]; +#define STRINPUT(size) "%" # size "s" + + FILE *out = fmemopen (pldd.out.buffer, pldd.out.length, "r"); + TEST_VERIFY (out != NULL); + + /* First line is in the form of : */ + TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2); + + TEST_COMPARE (pid, target.pid); + TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0); + + /* It expects only one loader and libc loaded by the program. */ + bool interpreter_found = false, libc_found = false; + while (fgets (buffer, array_length (buffer), out) != NULL) + { + /* Ignore vDSO. */ + if (buffer[0] != '/') + continue; + + /* Remove newline so baseline (buffer) can compare against the + LD_SO and LIBC_SO macros unmodified. */ + if (buffer[strlen(buffer)-1] == '\n') + buffer[strlen(buffer)-1] = '\0'; + + if (strcmp (basename (buffer), LD_SO) == 0) + { + TEST_COMPARE (interpreter_found, false); + interpreter_found = true; + continue; + } + + if (strcmp (basename (buffer), LIBC_SO) == 0) + { + TEST_COMPARE (libc_found, false); + libc_found = true; + continue; + } + } + TEST_COMPARE (interpreter_found, true); + TEST_COMPARE (libc_found, true); + + fclose (out); + } + + support_capture_subprocess_free (&pldd); + support_process_terminate (&target); + + return 0; +} + +#include commit 52b7cd6e9a701bb203023d56e84551943dc6a4c0 Author: Adam Maris Date: Thu Mar 14 16:51:16 2019 -0400 malloc: Check for large bin list corruption when inserting unsorted chunk Fixes bug 24216. This patch adds security checks for bk and bk_nextsize pointers of chunks in large bin when inserting chunk from unsorted bin. It was possible to write the pointer to victim (newly inserted chunk) to arbitrary memory locations if bk or bk_nextsize pointers of the next large bin chunk got corrupted. (cherry picked from commit 5b06f538c5aee0389ed034f60d90a8884d6d54de) diff --git a/malloc/malloc.c b/malloc/malloc.c index feaf7ee0bf..ce771375b6 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3876,10 +3876,14 @@ _int_malloc (mstate av, size_t bytes) { victim->fd_nextsize = fwd; victim->bk_nextsize = fwd->bk_nextsize; + if (__glibc_unlikely (fwd->bk_nextsize->fd_nextsize != fwd)) + malloc_printerr ("malloc(): largebin double linked list corrupted (nextsize)"); fwd->bk_nextsize = victim; victim->bk_nextsize->fd_nextsize = victim; } bck = fwd->bk; + if (bck->fd != fwd) + malloc_printerr ("malloc(): largebin double linked list corrupted (bk)"); } } else commit c6177be4b92d5d7df50a785652d1912db511423e Author: Andreas Schwab Date: Wed May 15 17:09:05 2019 +0200 Fix crash in _IO_wfile_sync (bug 20568) When computing the length of the converted part of the stdio buffer, use the number of consumed wide characters, not the (negative) distance to the end of the wide buffer. (cherry picked from commit 32ff397533715988c19cbf3675dcbd727ec13e18) diff --git a/ChangeLog b/ChangeLog index 5af8e27ab9..d3fcf73e47 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2019-05-15 Andreas Schwab + + [BZ #20568] + * libio/wfileops.c (_IO_wfile_sync): Correct last argument to + __codecvt_do_length. + * libio/Makefile (tests): Add tst-wfile-sync. + ($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales). + * libio/tst-wfile-sync.c: New file. + * libio/tst-wfile-sync.input: New file. + 2019-04-23 Adhemerval Zanella [BZ #18035] diff --git a/NEWS b/NEWS index b39a0ccf91..c10ab5e851 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,7 @@ The following bugs are resolved with this release: [16573] malloc: Set and reset all hooks for tracing [18035] Fix pldd hang + [20568] Fix crash in _IO_wfile_sync [24155] x32 memcmp can treat positive length as 0 (if sign bit in RDX is set) (CVE-2019-7309) [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 diff --git a/libio/Makefile b/libio/Makefile index 5bee83e55c..ee3a34cc3b 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -65,7 +65,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \ - tst-sprintf-ub tst-sprintf-chk-ub + tst-sprintf-ub tst-sprintf-chk-ub tst-wfile-sync tests-internal = tst-vtables tst-vtables-interposed tst-readline @@ -212,6 +212,7 @@ $(objpfx)tst-ungetwc1.out: $(gen-locales) $(objpfx)tst-ungetwc2.out: $(gen-locales) $(objpfx)tst-widetext.out: $(gen-locales) $(objpfx)tst_wprintf2.out: $(gen-locales) +$(objpfx)tst-wfile-sync.out: $(gen-locales) endif $(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen diff --git a/libio/tst-wfile-sync.c b/libio/tst-wfile-sync.c new file mode 100644 index 0000000000..618682064d --- /dev/null +++ b/libio/tst-wfile-sync.c @@ -0,0 +1,39 @@ +/* Test that _IO_wfile_sync does not crash (bug 20568). + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include + +static int +do_test (void) +{ + TEST_VERIFY_EXIT (setlocale (LC_ALL, "de_DE.UTF-8") != NULL); + /* Fill the stdio buffer and advance the read pointer. */ + TEST_VERIFY_EXIT (fgetwc (stdin) != WEOF); + /* This calls _IO_wfile_sync, it should not crash. */ + TEST_VERIFY_EXIT (setvbuf (stdin, NULL, _IONBF, 0) == 0); + /* Verify that the external file offset has been synchronized. */ + TEST_COMPARE (xlseek (0, 0, SEEK_CUR), 1); + + return 0; +} + +#include diff --git a/libio/tst-wfile-sync.input b/libio/tst-wfile-sync.input new file mode 100644 index 0000000000..12d0958f7a --- /dev/null +++ b/libio/tst-wfile-sync.input @@ -0,0 +1 @@ +This is a test of _IO_wfile_sync. diff --git a/libio/wfileops.c b/libio/wfileops.c index 78f20486e5..bab2ba4892 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -508,11 +508,12 @@ _IO_wfile_sync (FILE *fp) generate the wide characters up to the current reading position. */ int nread; - + size_t wnread = (fp->_wide_data->_IO_read_ptr + - fp->_wide_data->_IO_read_base); fp->_wide_data->_IO_state = fp->_wide_data->_IO_last_state; nread = (*cv->__codecvt_do_length) (cv, &fp->_wide_data->_IO_state, fp->_IO_read_base, - fp->_IO_read_end, delta); + fp->_IO_read_end, wnread); fp->_IO_read_ptr = fp->_IO_read_base + nread; delta = -(fp->_IO_read_end - fp->_IO_read_base - nread); } commit e3f828b8bd6e21922da8be8dee35edef09382d8d Author: Mark Wielaard Date: Wed May 15 17:14:01 2019 +0200 dlfcn: Guard __dlerror_main_freeres with __libc_once_get (once) [BZ#24476] dlerror.c (__dlerror_main_freeres) will try to free resources which only have been initialized when init () has been called. That function is called when resources are needed using __libc_once (once, init) where once is a __libc_once_define (static, once) in the dlerror.c file. Trying to free those resources if init () hasn't been called will produce errors under valgrind memcheck. So guard the freeing of those resources using __libc_once_get (once) and make sure we have a valid key. Also add a similar guard to __dlerror (). * dlfcn/dlerror.c (__dlerror_main_freeres): Guard using __libc_once_get (once) and static_bug == NULL. (__dlerror): Check we have a valid key, set result to static_buf otherwise. Reviewed-by: Carlos O'Donell (cherry picked from commit 11b451c8868d8a2b0edc5dfd44fc58d9ee538be0) diff --git a/ChangeLog b/ChangeLog index d3fcf73e47..de8c5c214d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2019-05-15 Mark Wielaard + + [BZ#24476] + * dlfcn/dlerror.c (__dlerror_main_freeres): Guard using + __libc_once_get (once) and static_buf == NULL. + (__dlerror): Check we have a valid key, set result to static_buf + otherwise. + 2019-05-15 Andreas Schwab [BZ #20568] diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c index 27376582d0..ca42c126c1 100644 --- a/dlfcn/dlerror.c +++ b/dlfcn/dlerror.c @@ -72,9 +72,16 @@ __dlerror (void) __libc_once (once, init); /* Get error string. */ - result = (struct dl_action_result *) __libc_getspecific (key); - if (result == NULL) - result = &last_result; + if (static_buf != NULL) + result = static_buf; + else + { + /* init () has been run and we don't use the static buffer. + So we have a valid key. */ + result = (struct dl_action_result *) __libc_getspecific (key); + if (result == NULL) + result = &last_result; + } /* Test whether we already returned the string. */ if (result->returned != 0) @@ -230,13 +237,19 @@ free_key_mem (void *mem) void __dlerror_main_freeres (void) { - void *mem; /* Free the global memory if used. */ check_free (&last_result); - /* Free the TSD memory if used. */ - mem = __libc_getspecific (key); - if (mem != NULL) - free_key_mem (mem); + + if (__libc_once_get (once) && static_buf == NULL) + { + /* init () has been run and we don't use the static buffer. + So we have a valid key. */ + void *mem; + /* Free the TSD memory if used. */ + mem = __libc_getspecific (key); + if (mem != NULL) + free_key_mem (mem); + } } struct dlfcn_hook *_dlfcn_hook __attribute__((nocommon)); commit 980cb43f15c4320afc6272e7a508f1415413b945 Author: Mark Wielaard Date: Thu May 16 15:25:40 2019 +0200 Update NEWS for BZ#24476 Guard __dlerror_main_freeres. diff --git a/NEWS b/NEWS index c10ab5e851..17dcaa7b74 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,7 @@ The following bugs are resolved with this release: [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 [24211] Use-after-free in Systemtap probe in pthread_join + [24476] dlfcn: Guard __dlerror_main_freeres with __libc_once_get (once) Security related changes: commit 95d66fecaabbc92ab53027e808f0fc1929c9f21a Author: Wilco Dijkstra Date: Fri May 10 16:38:21 2019 +0100 Fix tcache count maximum (BZ #24531) The tcache counts[] array is a char, which has a very small range and thus may overflow. When setting tcache_count tunable, there is no overflow check. However the tunable must not be larger than the maximum value of the tcache counts[] array, otherwise it can overflow when filling the tcache. [BZ #24531] * malloc/malloc.c (MAX_TCACHE_COUNT): New define. (do_set_tcache_count): Only update if count is small enough. * manual/tunables.texi (glibc.malloc.tcache_count): Document max value. (cherry picked from commit 5ad533e8e65092be962e414e0417112c65d154fb) diff --git a/ChangeLog b/ChangeLog index de8c5c214d..f4586d34a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2019-05-22 Wilco Dijkstra + + [BZ #24531] + * malloc/malloc.c (MAX_TCACHE_COUNT): New define. + (do_set_tcache_count): Only update if count is small enough. + * manual/tunables.texi (glibc.malloc.tcache_count): Document max value. + 2019-05-15 Mark Wielaard [BZ#24476] diff --git a/malloc/malloc.c b/malloc/malloc.c index ce771375b6..0abd653be2 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -2919,6 +2919,8 @@ typedef struct tcache_perthread_struct tcache_entry *entries[TCACHE_MAX_BINS]; } tcache_perthread_struct; +#define MAX_TCACHE_COUNT 127 /* Maximum value of counts[] entries. */ + static __thread bool tcache_shutting_down = false; static __thread tcache_perthread_struct *tcache = NULL; @@ -5124,8 +5126,11 @@ static inline int __always_inline do_set_tcache_count (size_t value) { - LIBC_PROBE (memory_tunable_tcache_count, 2, value, mp_.tcache_count); - mp_.tcache_count = value; + if (value <= MAX_TCACHE_COUNT) + { + LIBC_PROBE (memory_tunable_tcache_count, 2, value, mp_.tcache_count); + mp_.tcache_count = value; + } return 1; } diff --git a/manual/tunables.texi b/manual/tunables.texi index af820820e0..8edfea4edd 100644 --- a/manual/tunables.texi +++ b/manual/tunables.texi @@ -189,8 +189,8 @@ per-thread cache. The default (and maximum) value is 1032 bytes on @deftp Tunable glibc.malloc.tcache_count The maximum number of chunks of each size to cache. The default is 7. -There is no upper limit, other than available system memory. If set -to zero, the per-thread cache is effectively disabled. +The upper limit is 127. If set to zero, the per-thread cache is effectively +disabled. The approximate maximum overhead of the per-thread cache is thus equal to the number of bins times the chunk count in each bin times the size commit bd0a325b6a263ae9dd9107d3476b1a594f20382a Author: Florian Weimer Date: Wed Jun 5 14:02:05 2019 +0200 malloc: Fix warnings in tests with GCC 9 This is a partial backport of test changes in commit 9bf8e29ca136094f73f69f725f15c51facc97206 ("malloc: make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)"), without the actual functionality changes. diff --git a/ChangeLog b/ChangeLog index f4586d34a3..6e6ff73bfc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2019-04-18 Adhemerval Zanella + + * malloc/tst-memalign.c (do_test): Disable + -Walloc-size-larger-than= around tests of malloc with negative + sizes. + * malloc/tst-malloc-too-large.c (do_test): Likewise. + 2019-05-22 Wilco Dijkstra [BZ #24531] diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c index 15e25f558e..51d42cee81 100644 --- a/malloc/tst-malloc-too-large.c +++ b/malloc/tst-malloc-too-large.c @@ -72,13 +72,28 @@ test_large_allocations (size_t size) void * ptr_to_realloc; test_setup (); + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (malloc (size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); ptr_to_realloc = malloc (16); TEST_VERIFY_EXIT (ptr_to_realloc != NULL); test_setup (); +#if __GNUC_PREREQ (7, 0) + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (realloc (ptr_to_realloc, size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); free (ptr_to_realloc); @@ -135,7 +150,13 @@ test_large_aligned_allocations (size_t size) for (align = 1; align <= pagesize; align *= 2) { test_setup (); +#if __GNUC_PREREQ (7, 0) + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (memalign (align, size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); /* posix_memalign expects an alignment that is a power of 2 *and* a @@ -151,7 +172,13 @@ test_large_aligned_allocations (size_t size) if ((size % align) == 0) { test_setup (); +#if __GNUC_PREREQ (7, 0) + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (aligned_alloc (align, size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); } } @@ -159,11 +186,23 @@ test_large_aligned_allocations (size_t size) /* Both valloc and pvalloc return page-aligned memory. */ test_setup (); +#if __GNUC_PREREQ (7, 0) + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (valloc (size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); test_setup (); +#if __GNUC_PREREQ (7, 0) + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif TEST_VERIFY (pvalloc (size) == NULL); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif TEST_VERIFY (errno == ENOMEM); } diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c index a6a9140a3d..e7997518cb 100644 --- a/malloc/tst-memalign.c +++ b/malloc/tst-memalign.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors = 0; @@ -41,9 +42,18 @@ do_test (void) errno = 0; + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif /* An attempt to allocate a huge value should return NULL and set errno to ENOMEM. */ p = memalign (sizeof (void *), -1); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif save = errno; commit 2ec0b166bf04e22bb6921a08bb1a1131b57889a1 Author: Zack Weinberg Date: Mon Mar 11 10:59:27 2019 -0400 Use a proper C tokenizer to implement the obsolete typedefs test. The test for obsolete typedefs in installed headers was implemented using grep, and could therefore get false positives on e.g. “ulong” in a comment. It was also scanning all of the headers included by our headers, and therefore testing headers we don’t control, e.g. Linux kernel headers. This patch splits the obsolete-typedef test from scripts/check-installed-headers.sh to a separate program, scripts/check-obsolete-constructs.py. Being implemented in Python, it is feasible to make it tokenize C accurately enough to avoid false positives on the contents of comments and strings. It also only examines $(headers) in each subdirectory--all the headers we install, but not any external dependencies of those headers. Headers whose installed name starts with finclude/ are ignored, on the assumption that they contain Fortran. It is also feasible to make the new test understand the difference between _defining_ the obsolete typedefs and _using_ the obsolete typedefs, which means posix/{bits,sys}/types.h no longer need to be exempted. This uncovered an actual bug in bits/types.h: __quad_t and __u_quad_t were being used to define __S64_TYPE, __U64_TYPE, __SQUAD_TYPE and __UQUAD_TYPE. These are changed to __int64_t and __uint64_t respectively. This is a safe change, despite the comments in bits/types.h claiming a difference between __quad_t and __int64_t, because those comments are incorrect. In all current ABIs, both __quad_t and __int64_t are ‘long’ when ‘long’ is a 64-bit type, and ‘long long’ when ‘long’ is a 32-bit type, and similarly for __u_quad_t and __uint64_t. (Changing the types to be what the comments say they are would be an ABI break, as it affects C++ name mangling.) This patch includes a minimal change to make the comments not completely wrong. sys/types.h was defining the legacy BSD u_intN_t typedefs using a construct that was not necessarily consistent with how the C99 uintN_t typedefs are defined, and is also too complicated for the new script to understand (it lexes C relatively accurately, but it does not attempt to expand preprocessor macros, nor does it do any actual parsing). This patch cuts all of that out and uses bits/types.h's __uintN_t typedefs to define u_intN_t instead. This is verified to not change the ABI on any supported architecture, via the c++-types test, which means u_intN_t and uintN_t were, in fact, consistent on all supported architectures. Reviewed-by: Carlos O'Donell * scripts/check-obsolete-constructs.py: New test script. * scripts/check-installed-headers.sh: Remove tests for obsolete typedefs, superseded by check-obsolete-constructs.py. * Rules: Run scripts/check-obsolete-constructs.py over $(headers) as a special test. Update commentary. * posix/bits/types.h (__SQUAD_TYPE, __S64_TYPE): Define as __int64_t. (__UQUAD_TYPE, __U64_TYPE): Define as __uint64_t. Update commentary. * posix/sys/types.h (__u_intN_t): Remove. (u_int8_t): Typedef using __uint8_t. (u_int16_t): Typedef using __uint16_t. (u_int32_t): Typedef using __uint32_t. (u_int64_t): Typedef using __uint64_t. (cherry picked from commit 711a322a235d4c8177713f11aa59156603b94aeb) diff --git a/ChangeLog b/ChangeLog index 6e6ff73bfc..306ea16ad5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2019-03-13 Zack Weinberg + + * scripts/check-obsolete-constructs.py: New test script. + * scripts/check-installed-headers.sh: Remove tests for + obsolete typedefs, superseded by check-obsolete-constructs.py. + * Rules: Run scripts/check-obsolete-constructs.py over $(headers) + as a special test. Update commentary. + * posix/bits/types.h (__SQUAD_TYPE, __S64_TYPE): Define as __int64_t. + (__UQUAD_TYPE, __U64_TYPE): Define as __uint64_t. + Update commentary. + * posix/sys/types.h (__u_intN_t): Remove. + (u_int8_t): Typedef using __uint8_t. + (u_int16_t): Typedef using __uint16_t. + (u_int32_t): Typedef using __uint32_t. + (u_int64_t): Typedef using __uint64_t. + 2019-04-18 Adhemerval Zanella * malloc/tst-memalign.c (do_test): Disable diff --git a/Rules b/Rules index 1562f2ce6d..16afa6acaa 100644 --- a/Rules +++ b/Rules @@ -82,7 +82,8 @@ $(common-objpfx)dummy.c: common-generated += dummy.o dummy.c ifneq "$(headers)" "" -# Special test of all the installed headers in this directory. +# Test that all of the headers installed by this directory can be compiled +# in isolation. tests-special += $(objpfx)check-installed-headers-c.out libof-check-installed-headers-c := testsuite $(objpfx)check-installed-headers-c.out: \ @@ -93,6 +94,8 @@ $(objpfx)check-installed-headers-c.out: \ $(evaluate-test) ifneq "$(CXX)" "" +# If a C++ compiler is available, also test that they can be compiled +# in isolation as C++. tests-special += $(objpfx)check-installed-headers-cxx.out libof-check-installed-headers-cxx := testsuite $(objpfx)check-installed-headers-cxx.out: \ diff --git a/posix/bits/types.h b/posix/bits/types.h index 27e065c3be..0de6c59bb4 100644 --- a/posix/bits/types.h +++ b/posix/bits/types.h @@ -87,7 +87,7 @@ __extension__ typedef unsigned long long int __uintmax_t; 32 -- "natural" 32-bit type (always int) 64 -- "natural" 64-bit type (long or long long) LONG32 -- 32-bit type, traditionally long - QUAD -- 64-bit type, always long long + QUAD -- 64-bit type, traditionally long long WORD -- natural type of __WORDSIZE bits (int or long) LONGWORD -- type of __WORDSIZE bits, traditionally long @@ -113,14 +113,14 @@ __extension__ typedef unsigned long long int __uintmax_t; #define __SLONGWORD_TYPE long int #define __ULONGWORD_TYPE unsigned long int #if __WORDSIZE == 32 -# define __SQUAD_TYPE __quad_t -# define __UQUAD_TYPE __u_quad_t +# define __SQUAD_TYPE __int64_t +# define __UQUAD_TYPE __uint64_t # define __SWORD_TYPE int # define __UWORD_TYPE unsigned int # define __SLONG32_TYPE long int # define __ULONG32_TYPE unsigned long int -# define __S64_TYPE __quad_t -# define __U64_TYPE __u_quad_t +# define __S64_TYPE __int64_t +# define __U64_TYPE __uint64_t /* We want __extension__ before typedef's that use nonstandard base types such as `long long' in C89 mode. */ # define __STD_TYPE __extension__ typedef diff --git a/posix/sys/types.h b/posix/sys/types.h index 27129c5c23..0e37b1ce6a 100644 --- a/posix/sys/types.h +++ b/posix/sys/types.h @@ -154,37 +154,20 @@ typedef unsigned int uint; #include -#if !__GNUC_PREREQ (2, 7) - /* These were defined by ISO C without the first `_'. */ -typedef unsigned char u_int8_t; -typedef unsigned short int u_int16_t; -typedef unsigned int u_int32_t; -# if __WORDSIZE == 64 -typedef unsigned long int u_int64_t; -# else -__extension__ typedef unsigned long long int u_int64_t; -# endif - -typedef int register_t; - -#else - -/* For GCC 2.7 and later, we can use specific type-size attributes. */ -# define __u_intN_t(N, MODE) \ - typedef unsigned int u_int##N##_t __attribute__ ((__mode__ (MODE))) - -__u_intN_t (8, __QI__); -__u_intN_t (16, __HI__); -__u_intN_t (32, __SI__); -__u_intN_t (64, __DI__); +typedef __uint8_t u_int8_t; +typedef __uint16_t u_int16_t; +typedef __uint32_t u_int32_t; +typedef __uint64_t u_int64_t; +#if __GNUC_PREREQ (2, 7) typedef int register_t __attribute__ ((__mode__ (__word__))); - +#else +typedef int register_t; +#endif /* Some code from BIND tests this macro to see if the types above are defined. */ -#endif #define __BIT_TYPES_DEFINED__ 1 diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index 8e7beffd82..63bc8d4fa6 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -16,11 +16,9 @@ # License along with the GNU C Library; if not, see # . -# Check installed headers for cleanliness. For each header, confirm -# that it's possible to compile a file that includes that header and -# does nothing else, in several different compilation modes. Also, -# scan the header for a set of obsolete typedefs that should no longer -# appear. +# For each installed header, confirm that it's possible to compile a +# file that includes that header and does nothing else, in several +# different compilation modes. # These compilation switches assume GCC or compatible, which is probably # fine since we also assume that when _building_ glibc. @@ -31,13 +29,6 @@ cxx_modes="-std=c++98 -std=gnu++98 -std=c++11 -std=gnu++11" # These are probably the most commonly used three. lib_modes="-D_DEFAULT_SOURCE=1 -D_GNU_SOURCE=1 -D_XOPEN_SOURCE=700" -# sys/types.h+bits/types.h have to define the obsolete types. -# rpc(svc)/* have the obsolete types too deeply embedded in their API -# to remove. -skip_obsolete_type_check='*/sys/types.h|*/bits/types.h|*/rpc/*|*/rpcsvc/*' -obsolete_type_re=\ -'\<((__)?(quad_t|u(short|int|long|_(char|short|int([0-9]+_t)?|long|quad_t))))\>' - if [ $# -lt 3 ]; then echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 exit 2 @@ -46,14 +37,10 @@ case "$1" in (c) lang_modes="$c_modes" cih_test_c=$(mktemp ${TMPDIR-/tmp}/cih_test_XXXXXX.c) - already="$skip_obsolete_type_check" ;; (c++) lang_modes="$cxx_modes" cih_test_c=$(mktemp ${TMPDIR-/tmp}/cih_test_XXXXXX.cc) - # The obsolete-type check can be skipped for C++; it is - # sufficient to do it for C. - already="*" ;; (*) echo "usage: $0 c|c++ \"compile command\" header header header..." >&2 @@ -151,22 +138,8 @@ $expanded_lib_mode int avoid_empty_translation_unit; EOF if $cc_cmd -fsyntax-only $lang_mode "$cih_test_c" 2>&1 - then - includes=$($cc_cmd -fsyntax-only -H $lang_mode \ - "$cih_test_c" 2>&1 | sed -ne 's/^[.][.]* //p') - for h in $includes; do - # Don't repeat work. - eval 'case "$h" in ('"$already"') continue;; esac' - - if grep -qE "$obsolete_type_re" "$h"; then - echo "*** Obsolete types detected:" - grep -HE "$obsolete_type_re" "$h" - failed=1 - fi - already="$already|$h" - done - else - failed=1 + then : + else failed=1 fi done done diff --git a/scripts/check-obsolete-constructs.py b/scripts/check-obsolete-constructs.py new file mode 100755 index 0000000000..ce5c72251f --- /dev/null +++ b/scripts/check-obsolete-constructs.py @@ -0,0 +1,466 @@ +#! /usr/bin/python3 +# Copyright (C) 2019 Free Software Foundation, Inc. +# This file is part of the GNU C Library. +# +# The GNU C Library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# The GNU C Library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with the GNU C Library; if not, see +# . + +"""Verifies that installed headers do not use any obsolete constructs: + * legacy BSD typedefs superseded by : + ushort uint ulong u_char u_short u_int u_long u_intNN_t quad_t u_quad_t + (sys/types.h is allowed to _define_ these types, but not to use them + to define anything else). +""" + +import argparse +import collections +import re +import sys + +# Simplified lexical analyzer for C preprocessing tokens. +# Does not implement trigraphs. +# Does not implement backslash-newline in the middle of any lexical +# item other than a string literal. +# Does not implement universal-character-names in identifiers. +# Treats prefixed strings (e.g. L"...") as two tokens (L and "...") +# Accepts non-ASCII characters only within comments and strings. + +# Caution: The order of the outermost alternation matters. +# STRING must be before BAD_STRING, CHARCONST before BAD_CHARCONST, +# BLOCK_COMMENT before BAD_BLOCK_COM before PUNCTUATOR, and OTHER must +# be last. +# Caution: There should be no capturing groups other than the named +# captures in the outermost alternation. + +# For reference, these are all of the C punctuators as of C11: +# [ ] ( ) { } , ; ? ~ +# ! != * *= / /= ^ ^= = == +# # ## +# % %= %> %: %:%: +# & &= && +# | |= || +# + += ++ +# - -= -- -> +# . ... +# : :> +# < <% <: << <<= <= +# > >= >> >>= + +# The BAD_* tokens are not part of the official definition of pp-tokens; +# they match unclosed strings, character constants, and block comments, +# so that the regex engine doesn't have to backtrack all the way to the +# beginning of a broken construct and then emit dozens of junk tokens. + +PP_TOKEN_RE_ = re.compile(r""" + (?P \"(?:[^\"\\\r\n]|\\(?:[\r\n -~]|\r\n))*\") + |(?P \"(?:[^\"\\\r\n]|\\[ -~])*) + |(?P \'(?:[^\'\\\r\n]|\\(?:[\r\n -~]|\r\n))*\') + |(?P \'(?:[^\'\\\r\n]|\\[ -~])*) + |(?P /\*(?:\*(?!/)|[^*])*\*/) + |(?P /\*(?:\*(?!/)|[^*])*\*?) + |(?P //[^\r\n]*) + |(?P [_a-zA-Z][_a-zA-Z0-9]*) + |(?P \.?[0-9](?:[0-9a-df-oq-zA-DF-OQ-Z_.]|[eEpP][+-]?)*) + |(?P + [,;?~(){}\[\]] + | [!*/^=]=? + | \#\#? + | %(?:[=>]|:(?:%:)?)? + | &[=&]? + |\|[=|]? + |\+[=+]? + | -[=->]? + |\.(?:\.\.)? + | :>? + | <(?:[%:]|<(?:=|<=?)?)? + | >(?:=|>=?)?) + |(?P \\(?:\r|\n|\r\n)) + |(?P [ \t\n\r\v\f]+) + |(?P .) +""", re.DOTALL | re.VERBOSE) + +HEADER_NAME_RE_ = re.compile(r""" + < [^>\r\n]+ > + | " [^"\r\n]+ " +""", re.DOTALL | re.VERBOSE) + +ENDLINE_RE_ = re.compile(r"""\r|\n|\r\n""") + +# based on the sample code in the Python re documentation +Token_ = collections.namedtuple("Token", ( + "kind", "text", "line", "column", "context")) +Token_.__doc__ = """ + One C preprocessing token, comment, or chunk of whitespace. + 'kind' identifies the token type, which will be one of: + STRING, CHARCONST, BLOCK_COMMENT, LINE_COMMENT, IDENT, + PP_NUMBER, PUNCTUATOR, ESCNL, WHITESPACE, HEADER_NAME, + or OTHER. The BAD_* alternatives in PP_TOKEN_RE_ are + handled within tokenize_c, below. + + 'text' is the sequence of source characters making up the token; + no decoding whatsoever is performed. + + 'line' and 'column' give the position of the first character of the + token within the source file. They are both 1-based. + + 'context' indicates whether or not this token occurred within a + preprocessing directive; it will be None for running text, + '' for the leading '#' of a directive line (because '#' + all by itself on a line is a "null directive"), or the name of + the directive for tokens within a directive line, starting with + the IDENT for the name itself. +""" + +def tokenize_c(file_contents, reporter): + """Yield a series of Token objects, one for each preprocessing + token, comment, or chunk of whitespace within FILE_CONTENTS. + The REPORTER object is expected to have one method, + reporter.error(token, message), which will be called to + indicate a lexical error at the position of TOKEN. + If MESSAGE contains the four-character sequence '{!r}', that + is expected to be replaced by repr(token.text). + """ + + Token = Token_ + PP_TOKEN_RE = PP_TOKEN_RE_ + ENDLINE_RE = ENDLINE_RE_ + HEADER_NAME_RE = HEADER_NAME_RE_ + + line_num = 1 + line_start = 0 + pos = 0 + limit = len(file_contents) + directive = None + at_bol = True + while pos < limit: + if directive == "include": + mo = HEADER_NAME_RE.match(file_contents, pos) + if mo: + kind = "HEADER_NAME" + directive = "after_include" + else: + mo = PP_TOKEN_RE.match(file_contents, pos) + kind = mo.lastgroup + if kind != "WHITESPACE": + directive = "after_include" + else: + mo = PP_TOKEN_RE.match(file_contents, pos) + kind = mo.lastgroup + + text = mo.group() + line = line_num + column = mo.start() - line_start + adj_line_start = 0 + # only these kinds can contain a newline + if kind in ("WHITESPACE", "BLOCK_COMMENT", "LINE_COMMENT", + "STRING", "CHARCONST", "BAD_BLOCK_COM", "ESCNL"): + for tmo in ENDLINE_RE.finditer(text): + line_num += 1 + adj_line_start = tmo.end() + if adj_line_start: + line_start = mo.start() + adj_line_start + + # Track whether or not we are scanning a preprocessing directive. + if kind == "LINE_COMMENT" or (kind == "WHITESPACE" and adj_line_start): + at_bol = True + directive = None + else: + if kind == "PUNCTUATOR" and text == "#" and at_bol: + directive = "" + elif kind == "IDENT" and directive == "": + directive = text + at_bol = False + + # Report ill-formed tokens and rewrite them as their well-formed + # equivalents, so downstream processing doesn't have to know about them. + # (Rewriting instead of discarding provides better error recovery.) + if kind == "BAD_BLOCK_COM": + reporter.error(Token("BAD_BLOCK_COM", "", line, column+1, ""), + "unclosed block comment") + text += "*/" + kind = "BLOCK_COMMENT" + elif kind == "BAD_STRING": + reporter.error(Token("BAD_STRING", "", line, column+1, ""), + "unclosed string") + text += "\"" + kind = "STRING" + elif kind == "BAD_CHARCONST": + reporter.error(Token("BAD_CHARCONST", "", line, column+1, ""), + "unclosed char constant") + text += "'" + kind = "CHARCONST" + + tok = Token(kind, text, line, column+1, + "include" if directive == "after_include" else directive) + # Do not complain about OTHER tokens inside macro definitions. + # $ and @ appear in macros defined by headers intended to be + # included from assembly language, e.g. sysdeps/mips/sys/asm.h. + if kind == "OTHER" and directive != "define": + self.error(tok, "stray {!r} in program") + + yield tok + pos = mo.end() + +# +# Base and generic classes for individual checks. +# + +class ConstructChecker: + """Scan a stream of C preprocessing tokens and possibly report + problems with them. The REPORTER object passed to __init__ has + one method, reporter.error(token, message), which should be + called to indicate a problem detected at the position of TOKEN. + If MESSAGE contains the four-character sequence '{!r}' then that + will be replaced with a textual representation of TOKEN. + """ + def __init__(self, reporter): + self.reporter = reporter + + def examine(self, tok): + """Called once for each token in a header file. + Call self.reporter.error if a problem is detected. + """ + raise NotImplementedError + + def eof(self): + """Called once at the end of the stream. Subclasses need only + override this if it might have something to do.""" + pass + +class NoCheck(ConstructChecker): + """Generic checker class which doesn't do anything. Substitute this + class for a real checker when a particular check should be skipped + for some file.""" + + def examine(self, tok): + pass + +# +# Check for obsolete type names. +# + +# The obsolete type names we're looking for: +OBSOLETE_TYPE_RE_ = re.compile(r"""\A + (__)? + ( quad_t + | u(?: short | int | long + | _(?: char | short | int(?:[0-9]+_t)? | long | quad_t ))) +\Z""", re.VERBOSE) + +class ObsoleteNotAllowed(ConstructChecker): + """Don't allow any use of the obsolete typedefs.""" + def examine(self, tok): + if OBSOLETE_TYPE_RE_.match(tok.text): + self.reporter.error(tok, "use of {!r}") + +class ObsoletePrivateDefinitionsAllowed(ConstructChecker): + """Allow definitions of the private versions of the + obsolete typedefs; that is, 'typedef [anything] __obsolete;' + """ + def __init__(self, reporter): + super().__init__(reporter) + self.in_typedef = False + self.prev_token = None + + def examine(self, tok): + # bits/types.h hides 'typedef' in a macro sometimes. + if (tok.kind == "IDENT" + and tok.text in ("typedef", "__STD_TYPE") + and tok.context is None): + self.in_typedef = True + elif tok.kind == "PUNCTUATOR" and tok.text == ";" and self.in_typedef: + self.in_typedef = False + if self.prev_token.kind == "IDENT": + m = OBSOLETE_TYPE_RE_.match(self.prev_token.text) + if m and m.group(1) != "__": + self.reporter.error(self.prev_token, "use of {!r}") + self.prev_token = None + else: + self._check_prev() + + self.prev_token = tok + + def eof(self): + self._check_prev() + + def _check_prev(self): + if (self.prev_token is not None + and self.prev_token.kind == "IDENT" + and OBSOLETE_TYPE_RE_.match(self.prev_token.text)): + self.reporter.error(self.prev_token, "use of {!r}") + +class ObsoletePublicDefinitionsAllowed(ConstructChecker): + """Allow definitions of the public versions of the obsolete + typedefs. Only specific forms of definition are allowed: + + typedef __obsolete obsolete; // identifiers must agree + typedef __uintN_t u_intN_t; // N must agree + typedef unsigned long int ulong; + typedef unsigned short int ushort; + typedef unsigned int uint; + """ + def __init__(self, reporter): + super().__init__(reporter) + self.typedef_tokens = [] + + def examine(self, tok): + if tok.kind in ("WHITESPACE", "BLOCK_COMMENT", + "LINE_COMMENT", "NL", "ESCNL"): + pass + + elif (tok.kind == "IDENT" and tok.text == "typedef" + and tok.context is None): + if self.typedef_tokens: + self.reporter.error(tok, "typedef inside typedef") + self._reset() + self.typedef_tokens.append(tok) + + elif tok.kind == "PUNCTUATOR" and tok.text == ";": + self._finish() + + elif self.typedef_tokens: + self.typedef_tokens.append(tok) + + def eof(self): + self._reset() + + def _reset(self): + while self.typedef_tokens: + tok = self.typedef_tokens.pop(0) + if tok.kind == "IDENT" and OBSOLETE_TYPE_RE_.match(tok.text): + self.reporter.error(tok, "use of {!r}") + + def _finish(self): + if not self.typedef_tokens: return + if self.typedef_tokens[-1].kind == "IDENT": + m = OBSOLETE_TYPE_RE_.match(self.typedef_tokens[-1].text) + if m: + if self._permissible_public_definition(m): + self.typedef_tokens.clear() + self._reset() + + def _permissible_public_definition(self, m): + if m.group(1) == "__": return False + name = m.group(2) + toks = self.typedef_tokens + ntok = len(toks) + if ntok == 3 and toks[1].kind == "IDENT": + defn = toks[1].text + n = OBSOLETE_TYPE_RE_.match(defn) + if n and n.group(1) == "__" and n.group(2) == name: + return True + + if (name[:5] == "u_int" and name[-2:] == "_t" + and defn[:6] == "__uint" and defn[-2:] == "_t" + and name[5:-2] == defn[6:-2]): + return True + + return False + + if (name == "ulong" and ntok == 5 + and toks[1].kind == "IDENT" and toks[1].text == "unsigned" + and toks[2].kind == "IDENT" and toks[2].text == "long" + and toks[3].kind == "IDENT" and toks[3].text == "int"): + return True + + if (name == "ushort" and ntok == 5 + and toks[1].kind == "IDENT" and toks[1].text == "unsigned" + and toks[2].kind == "IDENT" and toks[2].text == "short" + and toks[3].kind == "IDENT" and toks[3].text == "int"): + return True + + if (name == "uint" and ntok == 4 + and toks[1].kind == "IDENT" and toks[1].text == "unsigned" + and toks[2].kind == "IDENT" and toks[2].text == "int"): + return True + + return False + +def ObsoleteTypedefChecker(reporter, fname): + """Factory: produce an instance of the appropriate + obsolete-typedef checker for FNAME.""" + + # The obsolete rpc/ and rpcsvc/ headers are allowed to use the + # obsolete types, because it would be more trouble than it's + # worth to remove them from headers that we intend to stop + # installing eventually anyway. + if (fname.startswith("rpc/") + or fname.startswith("rpcsvc/") + or "/rpc/" in fname + or "/rpcsvc/" in fname): + return NoCheck(reporter) + + # bits/types.h is allowed to define the __-versions of the + # obsolete types. + if (fname == "bits/types.h" + or fname.endswith("/bits/types.h")): + return ObsoletePrivateDefinitionsAllowed(reporter) + + # sys/types.h is allowed to use the __-versions of the + # obsolete types, but only to define the unprefixed versions. + if (fname == "sys/types.h" + or fname.endswith("/sys/types.h")): + return ObsoletePublicDefinitionsAllowed(reporter) + + return ObsoleteNotAllowed(reporter) + +# +# Master control +# + +class HeaderChecker: + """Perform all of the checks on each header. This is also the + "reporter" object expected by tokenize_c and ConstructChecker. + """ + def __init__(self): + self.fname = None + self.status = 0 + + def error(self, tok, message): + self.status = 1 + if '{!r}' in message: + message = message.format(tok.text) + sys.stderr.write("{}:{}:{}: error: {}\n".format( + self.fname, tok.line, tok.column, message)) + + def check(self, fname): + self.fname = fname + try: + with open(fname, "rt") as fp: + contents = fp.read() + except OSError as e: + sys.stderr.write("{}: {}\n".format(fname, e.strerror)) + self.status = 1 + return + + typedef_checker = ObsoleteTypedefChecker(self, self.fname) + + for tok in tokenize_c(contents, self): + typedef_checker.examine(tok) + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("headers", metavar="header", nargs="+", + help="one or more headers to scan for obsolete constructs") + args = ap.parse_args() + + checker = HeaderChecker() + for fname in args.headers: + # Headers whose installed name begins with "finclude/" contain + # Fortran, not C, and this program should completely ignore them. + if not (fname.startswith("finclude/") or "/finclude/" in fname): + checker.check(fname) + sys.exit(checker.status) + +main() commit 34fb5f61d3c3f4b8fc616ea259fa19168b58ecd4 Author: Dmitry V. Levin Date: Wed Feb 13 01:20:51 2019 +0000 libio: do not attempt to free wide buffers of legacy streams [BZ #24228] Commit a601b74d31ca086de38441d316a3dee24c866305 aka glibc-2.23~693 ("In preparation for fixing BZ#16734, fix failure in misc/tst-error1-mem when _G_HAVE_MMAP is turned off.") introduced a regression: _IO_unbuffer_all now invokes _IO_wsetb to free wide buffers of all files, including legacy standard files which are small statically allocated objects that do not have wide buffers and the _mode member, causing memory corruption. Another memory corruption in _IO_unbuffer_all happens when -1 is assigned to the _mode member of legacy standard files that do not have it. [BZ #24228] * libio/genops.c (_IO_unbuffer_all) [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)]: Do not attempt to free wide buffers and access _IO_FILE_complete members of legacy libio streams. * libio/tst-bz24228.c: New file. * libio/tst-bz24228.map: Likewise. * libio/Makefile [build-shared] (tests): Add tst-bz24228. [build-shared] (generated): Add tst-bz24228.mtrace and tst-bz24228.check. [run-built-tests && build-shared] (tests-special): Add $(objpfx)tst-bz24228-mem.out. (LDFLAGS-tst-bz24228, tst-bz24228-ENV): New variables. ($(objpfx)tst-bz24228-mem.out): New rule. (cherry picked from commit 21cc130b78a4db9113fb6695e2b951e697662440) diff --git a/ChangeLog b/ChangeLog index 306ea16ad5..20b650e026 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2019-06-20 Dmitry V. Levin + Florian Weimer + + [BZ #24228] + * libio/genops.c (_IO_unbuffer_all) + [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)]: Do not attempt to free wide + buffers and access _IO_FILE_complete members of legacy libio streams. + * libio/tst-bz24228.c: New file. + * libio/tst-bz24228.map: Likewise. + * libio/Makefile [build-shared] (tests): Add tst-bz24228. + [build-shared] (generated): Add tst-bz24228.mtrace and + tst-bz24228.check. + [run-built-tests && build-shared] (tests-special): Add + $(objpfx)tst-bz24228-mem.out. + (LDFLAGS-tst-bz24228, tst-bz24228-ENV): New variables. + ($(objpfx)tst-bz24228-mem.out): New rule. + 2019-03-13 Zack Weinberg * scripts/check-obsolete-constructs.py: New test script. diff --git a/NEWS b/NEWS index 17dcaa7b74..68d88b1036 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,7 @@ The following bugs are resolved with this release: [24164] Systemtap probes need to use "nr" constraint on 32-bit Arm [24161] __run_fork_handlers self-deadlocks in malloc/tst-mallocfork2 [24211] Use-after-free in Systemtap probe in pthread_join + [24228] old x86 applications that use legacy libio crash on exit [24476] dlfcn: Guard __dlerror_main_freeres with __libc_once_get (once) Security related changes: diff --git a/libio/Makefile b/libio/Makefile index ee3a34cc3b..9626a16b01 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -73,6 +73,9 @@ ifeq (yes,$(build-shared)) # Add test-fopenloc only if shared library is enabled since it depends on # shared localedata objects. tests += tst-fopenloc +# Add tst-bz24228 only if shared library is enabled since it can never meet its +# objective with static linking because the relevant code just is not there. +tests += tst-bz24228 endif test-srcs = test-freopen @@ -157,11 +160,14 @@ CFLAGS-tst_putwc.c += -DOBJPFX=\"$(objpfx)\" CFLAGS-tst-sprintf-ub.c += -Wno-restrict CFLAGS-tst-sprintf-chk-ub.c += -Wno-restrict +LDFLAGS-tst-bz24228 = -Wl,--version-script=tst-bz24228.map + tst_wprintf2-ARGS = "Some Text" test-fmemopen-ENV = MALLOC_TRACE=$(objpfx)test-fmemopen.mtrace tst-fopenloc-ENV = MALLOC_TRACE=$(objpfx)tst-fopenloc.mtrace tst-bz22415-ENV = MALLOC_TRACE=$(objpfx)tst-bz22415.mtrace +tst-bz24228-ENV = MALLOC_TRACE=$(objpfx)tst-bz24228.mtrace generated += test-fmemopen.mtrace test-fmemopen.check generated += tst-fopenloc.mtrace tst-fopenloc.check @@ -170,6 +176,7 @@ generated += tst-bz22415.mtrace tst-bz22415.check aux := fileops genops stdfiles stdio strops ifeq ($(build-shared),yes) +generated += tst-bz24228.mtrace tst-bz24228.check aux += oldfileops oldstdfiles endif @@ -184,7 +191,8 @@ tests-special += $(objpfx)test-freopen.out $(objpfx)test-fmemopen-mem.out \ ifeq (yes,$(build-shared)) # Run tst-fopenloc-cmp.out and tst-openloc-mem.out only if shared # library is enabled since they depend on tst-fopenloc.out. -tests-special += $(objpfx)tst-fopenloc-cmp.out $(objpfx)tst-fopenloc-mem.out +tests-special += $(objpfx)tst-fopenloc-cmp.out $(objpfx)tst-fopenloc-mem.out \ + $(objpfx)tst-bz24228-mem.out endif endif @@ -236,3 +244,7 @@ $(objpfx)tst-fopenloc-mem.out: $(objpfx)tst-fopenloc.out $(objpfx)tst-bz22415-mem.out: $(objpfx)tst-bz22415.out $(common-objpfx)malloc/mtrace $(objpfx)tst-bz22415.mtrace > $@; \ $(evaluate-test) + +$(objpfx)tst-bz24228-mem.out: $(objpfx)tst-bz24228.out + $(common-objpfx)malloc/mtrace $(objpfx)tst-bz24228.mtrace > $@; \ + $(evaluate-test) diff --git a/libio/genops.c b/libio/genops.c index 2a0d9b81df..11a15549e8 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -789,9 +789,16 @@ _IO_unbuffer_all (void) for (fp = (FILE *) _IO_list_all; fp; fp = fp->_chain) { + int legacy = 0; + +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) + if (__glibc_unlikely (_IO_vtable_offset (fp) != 0)) + legacy = 1; +#endif + if (! (fp->_flags & _IO_UNBUFFERED) /* Iff stream is un-orientated, it wasn't used. */ - && fp->_mode != 0) + && (legacy || fp->_mode != 0)) { #ifdef _IO_MTSAFE_IO int cnt; @@ -805,7 +812,7 @@ _IO_unbuffer_all (void) __sched_yield (); #endif - if (! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) + if (! legacy && ! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) { fp->_flags |= _IO_USER_BUF; @@ -816,7 +823,7 @@ _IO_unbuffer_all (void) _IO_SETBUF (fp, NULL, 0); - if (fp->_mode > 0) + if (! legacy && fp->_mode > 0) _IO_wsetb (fp, NULL, NULL, 0); #ifdef _IO_MTSAFE_IO @@ -827,7 +834,8 @@ _IO_unbuffer_all (void) /* Make sure that never again the wide char functions can be used. */ - fp->_mode = -1; + if (! legacy) + fp->_mode = -1; } #ifdef _IO_MTSAFE_IO diff --git a/libio/tst-bz24228.c b/libio/tst-bz24228.c new file mode 100644 index 0000000000..6a74500d47 --- /dev/null +++ b/libio/tst-bz24228.c @@ -0,0 +1,29 @@ +/* BZ #24228 check for memory corruption in legacy libio + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include + +static int +do_test (void) +{ + mtrace (); + return 0; +} + +#include diff --git a/libio/tst-bz24228.map b/libio/tst-bz24228.map new file mode 100644 index 0000000000..4383e0817d --- /dev/null +++ b/libio/tst-bz24228.map @@ -0,0 +1,5 @@ +# Hide the symbol from libc.so.6 to switch to the libio/oldfileops.c +# implementation when it is available for the architecture. +{ + local: _IO_stdin_used; +}; commit da347f4aa3a67e018f19e51a7ece6add87dba5cf Author: Florian Weimer Date: Fri Jun 28 09:39:21 2019 +0200 io: Remove copy_file_range emulation [BZ #24744] The kernel is evolving this interface (e.g., removal of the restriction on cross-device copies), and keeping up with that is difficult. Applications which need the function should run kernels which support the system call instead of relying on the imperfect glibc emulation. Reviewed-by: Adhemerval Zanella (cherry picked from commit 5a659ccc0ec217ab02a4c273a1f6d346a359560a) diff --git a/ChangeLog b/ChangeLog index 20b650e026..55a2bc589f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,45 @@ +2019-06-28 Florian Weimer + + [BZ #24744] + io: Remove the copy_file_range emulation. + * sysdeps/unix/sysv/linux/copy_file_range.c (copy_file_range): Do + not define and call copy_file_range_compat. + * io/Makefile (tests-static, tests-internal): Do not add + tst-copy_file_range-compat. + * io/copy_file_range-compat.c: Remove file. + * io/copy_file_range.c (copy_file_range): Define as stub. + * io/tst-copy_file_range-compat.c: Remove file. + * io/tst-copy_file_range.c (xdevfile): Remove variable. + (typical_sizes): Update comment. Remove 16K sizes. + (maximum_offset, maximum_offset_errno, maximum_offset_hard_limit): + Remove variables. + (find_maximum_offset, pipe_as_source, pipe_as_destination) + (delayed_write_failure_beginning, delayed_write_failure_end) + (cross_device_failure, enospc_failure_1, enospc_failure) + (oappend_failure): Remove functions. + (tests): Adjust test case list. + (do_test): Remove file system search code. Check for ENOSYS from + copy_file_range. Do not free xdevfile. + * manual/llio.texi (Copying File Data): Document ENOSYS error from + copy_file_range. Do not document the EXDEV error, which future + kernels may not report. Update the wording to reflect that + further errors are possible. + * sysdeps/unix/sysv/linux/alpha/kernel-features.h + [__LINUX_KERNEL_VERSION < 0x040D00] (__ASSUME_COPY_FILE_RANGE): Do + not undefine. + * sysdeps/unix/sysv/linux/arm/kernel-features.h + [__LINUX_KERNEL_VERSION < 0x040700] (__ASSUME_COPY_FILE_RANGE): + Likewise. + * sysdeps/unix/sysv/linux/kernel-features.h + [__LINUX_KERNEL_VERSION >= 0x040500] (__ASSUME_COPY_FILE_RANGE): + Remove definition. + * sysdeps/unix/sysv/linux/microblaze/kernel-features.h + [__LINUX_KERNEL_VERSION < 0x040A00] (__ASSUME_COPY_FILE_RANGE): Do + not undefine. + * sysdeps/unix/sysv/linux/sh/kernel-features.h + [__LINUX_KERNEL_VERSION < 0x040800] (__ASSUME_COPY_FILE_RANGE): + Likewise. + 2019-06-20 Dmitry V. Levin Florian Weimer diff --git a/NEWS b/NEWS index 68d88b1036..ae9f3795e5 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,14 @@ Major new features: * The entry for the new Japanese era has been added for ja_JP locale. +* The copy_file_range function fails with ENOSYS if the kernel does not + support the system call of the same name. Previously, user space + emulation was performed, but its behavior did not match the kernel + behavior, which was deemed too confusing. Applications which use the + copy_file_range function will have to be run on kernels which implement + the copy_file_range system call. Support for most architectures was added + in version 4.5 of the mainline Linux kernel. + The following bugs are resolved with this release: [16573] malloc: Set and reset all hooks for tracing @@ -22,6 +30,7 @@ The following bugs are resolved with this release: [24211] Use-after-free in Systemtap probe in pthread_join [24228] old x86 applications that use legacy libio crash on exit [24476] dlfcn: Guard __dlerror_main_freeres with __libc_once_get (once) + [24744] io: Remove the copy_file_range emulation Security related changes: diff --git a/io/Makefile b/io/Makefile index cfae00bbfb..70914d36a0 100644 --- a/io/Makefile +++ b/io/Makefile @@ -73,11 +73,6 @@ tests := test-utime test-stat test-stat2 test-lfs tst-getcwd \ tst-fts tst-fts-lfs tst-open-tmpfile \ tst-copy_file_range tst-getcwd-abspath \ -# This test includes the compat implementation of copy_file_range, -# which uses internal, unexported libc functions. -tests-static += tst-copy_file_range-compat -tests-internal += tst-copy_file_range-compat - # Likewise for statx, but we do not need static linking here. tests-internal += tst-statx diff --git a/io/copy_file_range-compat.c b/io/copy_file_range-compat.c deleted file mode 100644 index 58dbeef3e9..0000000000 --- a/io/copy_file_range-compat.c +++ /dev/null @@ -1,160 +0,0 @@ -/* Emulation of copy_file_range. - Copyright (C) 2017-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -/* The following macros should be defined before including this - file: - - COPY_FILE_RANGE_DECL Declaration specifiers for the function below. - COPY_FILE_RANGE Name of the function to define. */ - -#include -#include -#include -#include -#include -#include -#include - -COPY_FILE_RANGE_DECL -ssize_t -COPY_FILE_RANGE (int infd, __off64_t *pinoff, - int outfd, __off64_t *poutoff, - size_t length, unsigned int flags) -{ - if (flags != 0) - { - __set_errno (EINVAL); - return -1; - } - - { - struct stat64 instat; - struct stat64 outstat; - if (fstat64 (infd, &instat) != 0 || fstat64 (outfd, &outstat) != 0) - return -1; - if (S_ISDIR (instat.st_mode) || S_ISDIR (outstat.st_mode)) - { - __set_errno (EISDIR); - return -1; - } - if (!S_ISREG (instat.st_mode) || !S_ISREG (outstat.st_mode)) - { - /* We need a regular input file so that the we can seek - backwards in case of a write failure. */ - __set_errno (EINVAL); - return -1; - } - if (instat.st_dev != outstat.st_dev) - { - /* Cross-device copies are not supported. */ - __set_errno (EXDEV); - return -1; - } - } - - /* The output descriptor must not have O_APPEND set. */ - { - int flags = __fcntl (outfd, F_GETFL); - if (flags & O_APPEND) - { - __set_errno (EBADF); - return -1; - } - } - - /* Avoid an overflow in the result. */ - if (length > SSIZE_MAX) - length = SSIZE_MAX; - - /* Main copying loop. The buffer size is arbitrary and is a - trade-off between stack size consumption, cache usage, and - amortization of system call overhead. */ - size_t copied = 0; - char buf[8192]; - while (length > 0) - { - size_t to_read = length; - if (to_read > sizeof (buf)) - to_read = sizeof (buf); - - /* Fill the buffer. */ - ssize_t read_count; - if (pinoff == NULL) - read_count = read (infd, buf, to_read); - else - read_count = __libc_pread64 (infd, buf, to_read, *pinoff); - if (read_count == 0) - /* End of file reached prematurely. */ - return copied; - if (read_count < 0) - { - if (copied > 0) - /* Report the number of bytes copied so far. */ - return copied; - return -1; - } - if (pinoff != NULL) - *pinoff += read_count; - - /* Write the buffer part which was read to the destination. */ - char *end = buf + read_count; - for (char *p = buf; p < end; ) - { - ssize_t write_count; - if (poutoff == NULL) - write_count = write (outfd, p, end - p); - else - write_count = __libc_pwrite64 (outfd, p, end - p, *poutoff); - if (write_count < 0) - { - /* Adjust the input read position to match what we have - written, so that the caller can pick up after the - error. */ - size_t written = p - buf; - /* NB: This needs to be signed so that we can form the - negative value below. */ - ssize_t overread = read_count - written; - if (pinoff == NULL) - { - if (overread > 0) - { - /* We are on an error recovery path, so we - cannot deal with failure here. */ - int save_errno = errno; - (void) __libc_lseek64 (infd, -overread, SEEK_CUR); - __set_errno (save_errno); - } - } - else /* pinoff != NULL */ - *pinoff -= overread; - - if (copied + written > 0) - /* Report the number of bytes copied so far. */ - return copied + written; - return -1; - } - p += write_count; - if (poutoff != NULL) - *poutoff += write_count; - } /* Write loop. */ - - copied += read_count; - length -= read_count; - } - return copied; -} diff --git a/io/copy_file_range.c b/io/copy_file_range.c index 7b968be19d..59fb979773 100644 --- a/io/copy_file_range.c +++ b/io/copy_file_range.c @@ -1,4 +1,4 @@ -/* Generic implementation of copy_file_range. +/* Stub implementation of copy_file_range. Copyright (C) 2017-2019 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -16,7 +16,15 @@ License along with the GNU C Library; if not, see . */ -#define COPY_FILE_RANGE_DECL -#define COPY_FILE_RANGE copy_file_range +#include +#include -#include +ssize_t +copy_file_range (int infd, __off64_t *pinoff, + int outfd, __off64_t *poutoff, + size_t length, unsigned int flags) +{ + __set_errno (ENOSYS); + return -1; +} +stub_warning (copy_file_range) diff --git a/io/tst-copy_file_range-compat.c b/io/tst-copy_file_range-compat.c deleted file mode 100644 index fe6de8ac68..0000000000 --- a/io/tst-copy_file_range-compat.c +++ /dev/null @@ -1,30 +0,0 @@ -/* Test the fallback implementation of copy_file_range. - Copyright (C) 2017-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -/* Get the declaration of the official copy_of_range function. */ -#include - -/* Compile a local version of copy_file_range. */ -#define COPY_FILE_RANGE_DECL static -#define COPY_FILE_RANGE copy_file_range_compat -#include - -/* Re-use the test, but run it against copy_file_range_compat defined - above. */ -#define copy_file_range copy_file_range_compat -#include "tst-copy_file_range.c" diff --git a/io/tst-copy_file_range.c b/io/tst-copy_file_range.c index a5dcf3c1f6..a9237cb384 100644 --- a/io/tst-copy_file_range.c +++ b/io/tst-copy_file_range.c @@ -20,22 +20,15 @@ #include #include #include -#include -#include -#include #include #include #include #include #include -#include #include #include #include #include -#ifdef CLONE_NEWNS -# include -#endif /* Boolean flags which indicate whether to use pointers with explicit output flags. */ @@ -49,10 +42,6 @@ static int infd; static char *outfile; static int outfd; -/* Like the above, but on a different file system. xdevfile can be - NULL if no suitable file system has been found. */ -static char *xdevfile; - /* Input and output offsets. Set according to do_inoff and do_outoff before the test. The offsets themselves are always set to zero. */ @@ -61,13 +50,10 @@ static off64_t *pinoff; static off64_t outoff; static off64_t *poutoff; -/* These are a collection of copy sizes used in tests. The selection - takes into account that the fallback implementation uses an - internal buffer of 8192 bytes. */ +/* These are a collection of copy sizes used in tests. */ enum { maximum_size = 99999 }; static const int typical_sizes[] = - { 0, 1, 2, 3, 1024, 2048, 4096, 8191, 8192, 8193, 16383, 16384, 16385, - maximum_size }; + { 0, 1, 2, 3, 1024, 2048, 4096, 8191, 8192, 8193, maximum_size }; /* The random contents of this array can be used as a pattern to check for correct write operations. */ @@ -76,101 +62,6 @@ static unsigned char random_data[maximum_size]; /* The size chosen by the test harness. */ static int current_size; -/* Maximum writable file offset. Updated by find_maximum_offset - below. */ -static off64_t maximum_offset; - -/* Error code when crossing the offset. */ -static int maximum_offset_errno; - -/* If true: Writes which cross the limit will fail. If false: Writes - which cross the limit will result in a partial write. */ -static bool maximum_offset_hard_limit; - -/* Fills maximum_offset etc. above. Truncates outfd as a side - effect. */ -static void -find_maximum_offset (void) -{ - xftruncate (outfd, 0); - if (maximum_offset != 0) - return; - - uint64_t upper = -1; - upper >>= 1; /* Maximum of off64_t. */ - TEST_VERIFY ((off64_t) upper > 0); - TEST_VERIFY ((off64_t) (upper + 1) < 0); - if (lseek64 (outfd, upper, SEEK_SET) >= 0) - { - if (write (outfd, "", 1) == 1) - FAIL_EXIT1 ("created a file larger than the off64_t range"); - } - - uint64_t lower = 1024 * 1024; /* A reasonable minimum file size. */ - /* Loop invariant: writing at lower succeeds, writing at upper fails. */ - while (lower + 1 < upper) - { - uint64_t middle = (lower + upper) / 2; - if (test_verbose > 0) - printf ("info: %s: remaining test range %" PRIu64 " .. %" PRIu64 - ", probe at %" PRIu64 "\n", __func__, lower, upper, middle); - xftruncate (outfd, 0); - if (lseek64 (outfd, middle, SEEK_SET) >= 0 - && write (outfd, "", 1) == 1) - lower = middle; - else - upper = middle; - } - TEST_VERIFY (lower + 1 == upper); - maximum_offset = lower; - printf ("info: maximum writable file offset: %" PRIu64 " (%" PRIx64 ")\n", - lower, lower); - - /* Check that writing at the valid offset actually works. */ - xftruncate (outfd, 0); - xlseek (outfd, lower, SEEK_SET); - TEST_COMPARE (write (outfd, "", 1), 1); - - /* Cross the boundary with a two-byte write. This can either result - in a short write, or a failure. */ - xlseek (outfd, lower, SEEK_SET); - ssize_t ret = write (outfd, " ", 2); - if (ret < 0) - { - maximum_offset_errno = errno; - maximum_offset_hard_limit = true; - } - else - maximum_offset_hard_limit = false; - - /* Check that writing at the next offset actually fails. This also - obtains the expected errno value. */ - xftruncate (outfd, 0); - const char *action; - if (lseek64 (outfd, lower + 1, SEEK_SET) != 0) - { - if (write (outfd, "", 1) != -1) - FAIL_EXIT1 ("write to impossible offset %" PRIu64 " succeeded", - lower + 1); - action = "writing"; - int errno_copy = errno; - if (maximum_offset_hard_limit) - TEST_COMPARE (errno_copy, maximum_offset_errno); - else - maximum_offset_errno = errno_copy; - } - else - { - action = "seeking"; - maximum_offset_errno = errno; - } - printf ("info: %s out of range fails with %m (%d)\n", - action, maximum_offset_errno); - - xftruncate (outfd, 0); - xlseek (outfd, 0, SEEK_SET); -} - /* Perform a copy of a file. */ static void simple_file_copy (void) @@ -247,390 +138,6 @@ simple_file_copy (void) free (bytes); } -/* Test that reading from a pipe willfails. */ -static void -pipe_as_source (void) -{ - int pipefds[2]; - xpipe (pipefds); - - for (int length = 0; length < 2; ++length) - { - if (test_verbose > 0) - printf ("info: %s: length=%d\n", __func__, length); - - /* Make sure that there is something to copy in the pipe. */ - xwrite (pipefds[1], "@", 1); - - TEST_COMPARE (copy_file_range (pipefds[0], pinoff, outfd, poutoff, - length, 0), -1); - /* Linux 4.10 and later return EINVAL. Older kernels return - EXDEV. */ - TEST_VERIFY (errno == EINVAL || errno == EXDEV); - TEST_COMPARE (inoff, 0); - TEST_COMPARE (outoff, 0); - TEST_COMPARE (xlseek (outfd, 0, SEEK_CUR), 0); - - /* Make sure that nothing was read. */ - char buf = 'A'; - TEST_COMPARE (read (pipefds[0], &buf, 1), 1); - TEST_COMPARE (buf, '@'); - } - - xclose (pipefds[0]); - xclose (pipefds[1]); -} - -/* Test that writing to a pipe fails. */ -static void -pipe_as_destination (void) -{ - /* Make sure that there is something to read in the input file. */ - xwrite (infd, "abc", 3); - xlseek (infd, 0, SEEK_SET); - - int pipefds[2]; - xpipe (pipefds); - - for (int length = 0; length < 2; ++length) - { - if (test_verbose > 0) - printf ("info: %s: length=%d\n", __func__, length); - - TEST_COMPARE (copy_file_range (infd, pinoff, pipefds[1], poutoff, - length, 0), -1); - /* Linux 4.10 and later return EINVAL. Older kernels return - EXDEV. */ - TEST_VERIFY (errno == EINVAL || errno == EXDEV); - TEST_COMPARE (inoff, 0); - TEST_COMPARE (outoff, 0); - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 0); - - /* Make sure that nothing was written. */ - struct pollfd pollfd = { .fd = pipefds[0], .events = POLLIN, }; - TEST_COMPARE (poll (&pollfd, 1, 0), 0); - } - - xclose (pipefds[0]); - xclose (pipefds[1]); -} - -/* Test a write failure after (potentially) writing some bytes. - Failure occurs near the start of the buffer. */ -static void -delayed_write_failure_beginning (void) -{ - /* We need to write something to provoke the error. */ - if (current_size == 0) - return; - xwrite (infd, random_data, sizeof (random_data)); - xlseek (infd, 0, SEEK_SET); - - /* Write failure near the start. The actual error code varies among - file systems. */ - find_maximum_offset (); - off64_t where = maximum_offset; - - if (current_size == 1) - ++where; - outoff = where; - if (do_outoff) - xlseek (outfd, 1, SEEK_SET); - else - xlseek (outfd, where, SEEK_SET); - if (maximum_offset_hard_limit || where > maximum_offset) - { - TEST_COMPARE (copy_file_range (infd, pinoff, outfd, poutoff, - sizeof (random_data), 0), -1); - TEST_COMPARE (errno, maximum_offset_errno); - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 0); - TEST_COMPARE (inoff, 0); - if (do_outoff) - TEST_COMPARE (xlseek (outfd, 0, SEEK_CUR), 1); - else - TEST_COMPARE (xlseek (outfd, 0, SEEK_CUR), where); - TEST_COMPARE (outoff, where); - struct stat64 st; - xfstat (outfd, &st); - TEST_COMPARE (st.st_size, 0); - } - else - { - /* The offset is not a hard limit. This means we write one - byte. */ - TEST_COMPARE (copy_file_range (infd, pinoff, outfd, poutoff, - sizeof (random_data), 0), 1); - if (do_inoff) - { - TEST_COMPARE (inoff, 1); - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 0); - } - else - { - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 1); - TEST_COMPARE (inoff, 0); - } - if (do_outoff) - { - TEST_COMPARE (xlseek (outfd, 0, SEEK_CUR), 1); - TEST_COMPARE (outoff, where + 1); - } - else - { - TEST_COMPARE (xlseek (outfd, 0, SEEK_CUR), where + 1); - TEST_COMPARE (outoff, where); - } - struct stat64 st; - xfstat (outfd, &st); - TEST_COMPARE (st.st_size, where + 1); - } -} - -/* Test a write failure after (potentially) writing some bytes. - Failure occurs near the end of the buffer. */ -static void -delayed_write_failure_end (void) -{ - if (current_size <= 1) - /* This would be same as the first test because there is not - enough data to write to make a difference. */ - return; - xwrite (infd, random_data, sizeof (random_data)); - xlseek (infd, 0, SEEK_SET); - - find_maximum_offset (); - off64_t where = maximum_offset - current_size + 1; - if (current_size == sizeof (random_data)) - /* Otherwise we do not reach the non-writable byte. */ - ++where; - outoff = where; - if (do_outoff) - xlseek (outfd, 1, SEEK_SET); - else - xlseek (outfd, where, SEEK_SET); - ssize_t ret = copy_file_range (infd, pinoff, outfd, poutoff, - sizeof (random_data), 0); - if (ret < 0) - { - TEST_COMPARE (ret, -1); - TEST_COMPARE (errno, maximum_offset_errno); - struct stat64 st; - xfstat (outfd, &st); - TEST_COMPARE (st.st_size, 0); - } - else - { - /* The first copy succeeded. This happens in the emulation - because the internal buffer of limited size does not - necessarily cross the off64_t boundary on the first write - operation. */ - if (test_verbose > 0) - printf ("info: copy_file_range (%zu) returned %zd\n", - sizeof (random_data), ret); - TEST_VERIFY (ret > 0); - TEST_VERIFY (ret < maximum_size); - struct stat64 st; - xfstat (outfd, &st); - TEST_COMPARE (st.st_size, where + ret); - if (do_inoff) - { - TEST_COMPARE (inoff, ret); - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 0); - } - else - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), ret); - - char *buffer = xmalloc (ret); - TEST_COMPARE (pread64 (outfd, buffer, ret, where), ret); - TEST_VERIFY (memcmp (buffer, random_data, ret) == 0); - free (buffer); - - /* The second copy fails. */ - TEST_COMPARE (copy_file_range (infd, pinoff, outfd, poutoff, - sizeof (random_data), 0), -1); - TEST_COMPARE (errno, maximum_offset_errno); - } -} - -/* Test a write failure across devices. */ -static void -cross_device_failure (void) -{ - if (xdevfile == NULL) - /* Subtest not supported due to missing cross-device file. */ - return; - - /* We need something to write. */ - xwrite (infd, random_data, sizeof (random_data)); - xlseek (infd, 0, SEEK_SET); - - int xdevfd = xopen (xdevfile, O_RDWR | O_LARGEFILE, 0); - TEST_COMPARE (copy_file_range (infd, pinoff, xdevfd, poutoff, - current_size, 0), -1); - TEST_COMPARE (errno, EXDEV); - TEST_COMPARE (xlseek (infd, 0, SEEK_CUR), 0); - struct stat64 st; - xfstat (xdevfd, &st); - TEST_COMPARE (st.st_size, 0); - - xclose (xdevfd); -} - -/* Try to exercise ENOSPC behavior with a tempfs file system (so that - we do not have to fill up a regular file system to get the error). - This function runs in a subprocess, so that we do not change the - mount namespace of the actual test process. */ -static void -enospc_failure_1 (void *closure) -{ -#ifdef CLONE_NEWNS - support_become_root (); - - /* Make sure that we do not alter the file system mounts of the - parents. */ - if (! support_enter_mount_namespace ()) - { - printf ("warning: ENOSPC test skipped\n"); - return; - } - - char *mountpoint = closure; - if (mount ("none", mountpoint, "tmpfs", MS_NODEV | MS_NOEXEC, - "size=500k") != 0) - { - printf ("warning: could not mount tmpfs at %s: %m\n", mountpoint); - return; - } - - /* The source file must reside on the same file system. */ - char *intmpfsfile = xasprintf ("%s/%s", mountpoint, "in"); - int intmpfsfd = xopen (intmpfsfile, O_RDWR | O_CREAT | O_LARGEFILE, 0600); - xwrite (intmpfsfd, random_data, sizeof (random_data)); - xlseek (intmpfsfd, 1, SEEK_SET); - inoff = 1; - - char *outtmpfsfile = xasprintf ("%s/%s", mountpoint, "out"); - int outtmpfsfd = xopen (outtmpfsfile, O_RDWR | O_CREAT | O_LARGEFILE, 0600); - - /* Fill the file with data until ENOSPC is reached. */ - while (true) - { - ssize_t ret = write (outtmpfsfd, random_data, sizeof (random_data)); - if (ret < 0 && errno != ENOSPC) - FAIL_EXIT1 ("write to %s: %m", outtmpfsfile); - if (ret < sizeof (random_data)) - break; - } - TEST_COMPARE (write (outtmpfsfd, "", 1), -1); - TEST_COMPARE (errno, ENOSPC); - off64_t maxsize = xlseek (outtmpfsfd, 0, SEEK_CUR); - TEST_VERIFY_EXIT (maxsize > sizeof (random_data)); - - /* Constructed the expected file contents. */ - char *expected = xmalloc (maxsize); - TEST_COMPARE (pread64 (outtmpfsfd, expected, maxsize, 0), maxsize); - /* Go back a little, so some bytes can be written. */ - enum { offset = 20000 }; - TEST_VERIFY_EXIT (offset < maxsize); - TEST_VERIFY_EXIT (offset < sizeof (random_data)); - memcpy (expected + maxsize - offset, random_data + 1, offset); - - if (do_outoff) - { - outoff = maxsize - offset; - xlseek (outtmpfsfd, 2, SEEK_SET); - } - else - xlseek (outtmpfsfd, -offset, SEEK_CUR); - - /* First call is expected to succeed because we made room for some - bytes. */ - TEST_COMPARE (copy_file_range (intmpfsfd, pinoff, outtmpfsfd, poutoff, - maximum_size, 0), offset); - if (do_inoff) - { - TEST_COMPARE (inoff, 1 + offset); - TEST_COMPARE (xlseek (intmpfsfd, 0, SEEK_CUR), 1); - } - else - TEST_COMPARE (xlseek (intmpfsfd, 0, SEEK_CUR), 1 + offset); - if (do_outoff) - { - TEST_COMPARE (outoff, maxsize); - TEST_COMPARE (xlseek (outtmpfsfd, 0, SEEK_CUR), 2); - } - else - TEST_COMPARE (xlseek (outtmpfsfd, 0, SEEK_CUR), maxsize); - struct stat64 st; - xfstat (outtmpfsfd, &st); - TEST_COMPARE (st.st_size, maxsize); - char *actual = xmalloc (st.st_size); - TEST_COMPARE (pread64 (outtmpfsfd, actual, st.st_size, 0), st.st_size); - TEST_VERIFY (memcmp (expected, actual, maxsize) == 0); - - /* Second call should fail with ENOSPC. */ - TEST_COMPARE (copy_file_range (intmpfsfd, pinoff, outtmpfsfd, poutoff, - maximum_size, 0), -1); - TEST_COMPARE (errno, ENOSPC); - - /* Offsets should be unchanged. */ - if (do_inoff) - { - TEST_COMPARE (inoff, 1 + offset); - TEST_COMPARE (xlseek (intmpfsfd, 0, SEEK_CUR), 1); - } - else - TEST_COMPARE (xlseek (intmpfsfd, 0, SEEK_CUR), 1 + offset); - if (do_outoff) - { - TEST_COMPARE (outoff, maxsize); - TEST_COMPARE (xlseek (outtmpfsfd, 0, SEEK_CUR), 2); - } - else - TEST_COMPARE (xlseek (outtmpfsfd, 0, SEEK_CUR), maxsize); - TEST_COMPARE (xlseek (outtmpfsfd, 0, SEEK_END), maxsize); - TEST_COMPARE (pread64 (outtmpfsfd, actual, maxsize, 0), maxsize); - TEST_VERIFY (memcmp (expected, actual, maxsize) == 0); - - free (actual); - free (expected); - - xclose (intmpfsfd); - xclose (outtmpfsfd); - free (intmpfsfile); - free (outtmpfsfile); - -#else /* !CLONE_NEWNS */ - puts ("warning: ENOSPC test skipped (no mount namespaces)"); -#endif -} - -/* Call enospc_failure_1 in a subprocess. */ -static void -enospc_failure (void) -{ - char *mountpoint - = support_create_temp_directory ("tst-copy_file_range-enospc-"); - support_isolate_in_subprocess (enospc_failure_1, mountpoint); - free (mountpoint); -} - -/* The target file descriptor must have O_APPEND enabled. */ -static void -oappend_failure (void) -{ - /* Add data, to make sure we do not fail because there is - insufficient input data. */ - xwrite (infd, random_data, current_size); - xlseek (infd, 0, SEEK_SET); - - xclose (outfd); - outfd = xopen (outfile, O_RDWR | O_APPEND, 0); - TEST_COMPARE (copy_file_range (infd, pinoff, outfd, poutoff, - current_size, 0), -1); - TEST_COMPARE (errno, EBADF); -} - /* Test that a short input file results in a shortened copy. */ static void short_copy (void) @@ -721,14 +228,6 @@ struct test_case static struct test_case tests[] = { { "simple_file_copy", simple_file_copy, .sizes = true }, - { "pipe_as_source", pipe_as_source, }, - { "pipe_as_destination", pipe_as_destination, }, - { "delayed_write_failure_beginning", delayed_write_failure_beginning, - .sizes = true }, - { "delayed_write_failure_end", delayed_write_failure_end, .sizes = true }, - { "cross_device_failure", cross_device_failure, .sizes = true }, - { "enospc_failure", enospc_failure, }, - { "oappend_failure", oappend_failure, .sizes = true }, { "short_copy", short_copy, .sizes = true }, }; @@ -739,58 +238,18 @@ do_test (void) *p = rand () >> 24; infd = create_temp_file ("tst-copy_file_range-in-", &infile); + outfd = create_temp_file ("tst-copy_file_range-out-", &outfile); { - int outfd = create_temp_file ("tst-copy_file_range-out-", &outfile); - if (!support_descriptor_supports_holes (outfd)) - FAIL_UNSUPPORTED ("File %s does not support holes", outfile); - xclose (outfd); - } - - /* Try to find a different directory from the default input/output - file. */ - { - struct stat64 instat; - xfstat (infd, &instat); - static const char *const candidates[] = - { NULL, "/var/tmp", "/dev/shm" }; - for (const char *const *c = candidates; c < array_end (candidates); ++c) - { - const char *path = *c; - char *to_free = NULL; - if (path == NULL) - { - to_free = xreadlink ("/proc/self/exe"); - path = dirname (to_free); - } - - struct stat64 cstat; - xstat (path, &cstat); - if (cstat.st_dev == instat.st_dev) - { - free (to_free); - continue; - } - - printf ("info: using alternate temporary files directory: %s\n", path); - xdevfile = xasprintf ("%s/tst-copy_file_range-xdev-XXXXXX", path); - free (to_free); - break; - } - if (xdevfile != NULL) + ssize_t ret = copy_file_range (infd, NULL, outfd, NULL, 0, 0); + if (ret != 0) { - int xdevfd = mkstemp (xdevfile); - if (xdevfd < 0) - FAIL_EXIT1 ("mkstemp (\"%s\"): %m", xdevfile); - struct stat64 xdevst; - xfstat (xdevfd, &xdevst); - TEST_VERIFY (xdevst.st_dev != instat.st_dev); - add_temp_file (xdevfile); - xclose (xdevfd); + if (errno == ENOSYS) + FAIL_UNSUPPORTED ("copy_file_range is not support on this system"); + FAIL_EXIT1 ("copy_file_range probing call: %m"); } - else - puts ("warning: no alternate directory on different file system found"); } xclose (infd); + xclose (outfd); for (do_inoff = 0; do_inoff < 2; ++do_inoff) for (do_outoff = 0; do_outoff < 2; ++do_outoff) @@ -832,7 +291,6 @@ do_test (void) free (infile); free (outfile); - free (xdevfile); return 0; } diff --git a/manual/llio.texi b/manual/llio.texi index 46b891d194..5b75676e94 100644 --- a/manual/llio.texi +++ b/manual/llio.texi @@ -1404,10 +1404,13 @@ failure occurs. The return value is zero if the end of the input file is encountered immediately. If no bytes can be copied, to report an error, @code{copy_file_range} -returns the value @math{-1} and sets @code{errno}. The following -@code{errno} error conditions are specific to this function: +returns the value @math{-1} and sets @code{errno}. The table below +lists some of the error conditions for this function. @table @code +@item ENOSYS +The kernel does not implement the required functionality. + @item EISDIR At least one of the descriptors @var{inputfd} or @var{outputfd} refers to a directory. @@ -1437,9 +1440,6 @@ reading. The argument @var{outputfd} is not a valid file descriptor open for writing, or @var{outputfd} has been opened with @code{O_APPEND}. - -@item EXDEV -The input and output files reside on different file systems. @end table In addition, @code{copy_file_range} can fail with the error codes diff --git a/sysdeps/unix/sysv/linux/alpha/kernel-features.h b/sysdeps/unix/sysv/linux/alpha/kernel-features.h index 0b0a94af2c..46178c5636 100644 --- a/sysdeps/unix/sysv/linux/alpha/kernel-features.h +++ b/sysdeps/unix/sysv/linux/alpha/kernel-features.h @@ -45,7 +45,6 @@ /* Support for copy_file_range, statx was added in kernel 4.13. */ #if __LINUX_KERNEL_VERSION < 0x040D00 # undef __ASSUME_MLOCK2 -# undef __ASSUME_COPY_FILE_RANGE # undef __ASSUME_STATX #endif diff --git a/sysdeps/unix/sysv/linux/arm/kernel-features.h b/sysdeps/unix/sysv/linux/arm/kernel-features.h index 2d2d355844..4220adff37 100644 --- a/sysdeps/unix/sysv/linux/arm/kernel-features.h +++ b/sysdeps/unix/sysv/linux/arm/kernel-features.h @@ -45,7 +45,6 @@ present in 32-bit kernels from 4.4 and 4.5 respectively. */ #if __LINUX_KERNEL_VERSION < 0x040700 # undef __ASSUME_MLOCK2 -# undef __ASSUME_COPY_FILE_RANGE #endif #undef __ASSUME_CLONE_DEFAULT diff --git a/sysdeps/unix/sysv/linux/copy_file_range.c b/sysdeps/unix/sysv/linux/copy_file_range.c index 70961007a5..e950db3bf5 100644 --- a/sysdeps/unix/sysv/linux/copy_file_range.c +++ b/sysdeps/unix/sysv/linux/copy_file_range.c @@ -20,27 +20,16 @@ #include #include -/* Include the fallback implementation. */ -#ifndef __ASSUME_COPY_FILE_RANGE -#define COPY_FILE_RANGE_DECL static -#define COPY_FILE_RANGE copy_file_range_compat -#include -#endif - ssize_t copy_file_range (int infd, __off64_t *pinoff, int outfd, __off64_t *poutoff, size_t length, unsigned int flags) { #ifdef __NR_copy_file_range - ssize_t ret = SYSCALL_CANCEL (copy_file_range, infd, pinoff, outfd, poutoff, - length, flags); -# ifndef __ASSUME_COPY_FILE_RANGE - if (ret == -1 && errno == ENOSYS) - ret = copy_file_range_compat (infd, pinoff, outfd, poutoff, length, flags); -# endif - return ret; -#else /* !__NR_copy_file_range */ - return copy_file_range_compat (infd, pinoff, outfd, poutoff, length, flags); + return SYSCALL_CANCEL (copy_file_range, infd, pinoff, outfd, poutoff, + length, flags); +#else + __set_errno (ENOSYS); + return -1; #endif } diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h index 0db1b987a4..abe98522dd 100644 --- a/sysdeps/unix/sysv/linux/kernel-features.h +++ b/sysdeps/unix/sysv/linux/kernel-features.h @@ -103,10 +103,6 @@ # define __ASSUME_MLOCK2 1 #endif -#if __LINUX_KERNEL_VERSION >= 0x040500 -# define __ASSUME_COPY_FILE_RANGE 1 -#endif - /* Support for statx was added in kernel 4.11. */ #if __LINUX_KERNEL_VERSION >= 0x040B00 # define __ASSUME_STATX 1 diff --git a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h index 8df19400af..a787409295 100644 --- a/sysdeps/unix/sysv/linux/microblaze/kernel-features.h +++ b/sysdeps/unix/sysv/linux/microblaze/kernel-features.h @@ -60,11 +60,6 @@ # undef __ASSUME_MLOCK2 #endif -/* Support for the copy_file_range syscall was added in 4.10. */ -#if __LINUX_KERNEL_VERSION < 0x040A00 -# undef __ASSUME_COPY_FILE_RANGE -#endif - /* Support for statx was added in kernel 4.12. */ #if __LINUX_KERNEL_VERSION < 0X040C00 # undef __ASSUME_STATX diff --git a/sysdeps/unix/sysv/linux/sh/kernel-features.h b/sysdeps/unix/sysv/linux/sh/kernel-features.h index 767df721b8..22cdc8049c 100644 --- a/sysdeps/unix/sysv/linux/sh/kernel-features.h +++ b/sysdeps/unix/sysv/linux/sh/kernel-features.h @@ -49,7 +49,6 @@ # undef __ASSUME_RENAMEAT2 # undef __ASSUME_EXECVEAT # undef __ASSUME_MLOCK2 -# undef __ASSUME_COPY_FILE_RANGE #endif /* sh does not support the statx system call. */ commit a00dc2a18f6c092ed4922a2fda92ebf179b2acde Author: Florian Weimer Date: Tue Jul 9 10:34:07 2019 +0200 NEWS: Add deprecated section heading diff --git a/NEWS b/NEWS index ae9f3795e5..21520414d7 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ Major new features: * The entry for the new Japanese era has been added for ja_JP locale. +Deprecated and removed features, and other changes affecting compatibility: + * The copy_file_range function fails with ENOSYS if the kernel does not support the system call of the same name. Previously, user space emulation was performed, but its behavior did not match the kernel