| Message ID | 20241121230047.2997929-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: Add smoke test for pthread_getcpuclockid failure | expand |
* Siddhesh Poyarekar: > +int > +do_test (void) > +{ > + pthread_t t = xpthread_create (NULL, thr, NULL); > + xpthread_join (t); > + > + clockid_t c; > + TEST_COMPARE (pthread_getcpuclockid (t, &c), ESRCH); This test is invalid because the lifetime of t has ended. This use-after-free bug becomes visible if you run the test with GLIBC_TUNABLES=glibc.pthread.stack_cache_size=0. Thanks, Florian
On 2024-11-22 02:42, Florian Weimer wrote: >> +int >> +do_test (void) >> +{ >> + pthread_t t = xpthread_create (NULL, thr, NULL); >> + xpthread_join (t); >> + >> + clockid_t c; >> + TEST_COMPARE (pthread_getcpuclockid (t, &c), ESRCH); > > This test is invalid because the lifetime of t has ended. This > use-after-free bug becomes visible if you run the test with > GLIBC_TUNABLES=glibc.pthread.stack_cache_size=0. Eek, of course, sorry I should have seen that :/ I can't see a way to intercept points within the lifetime of the thread id where tid is <=0, but I'll send an update if I do. Thanks, Sid
* Siddhesh Poyarekar: > On 2024-11-22 02:42, Florian Weimer wrote: >>> +int >>> +do_test (void) >>> +{ >>> + pthread_t t = xpthread_create (NULL, thr, NULL); >>> + xpthread_join (t); >>> + >>> + clockid_t c; >>> + TEST_COMPARE (pthread_getcpuclockid (t, &c), ESRCH); >> This test is invalid because the lifetime of t has ended. This >> use-after-free bug becomes visible if you run the test with >> GLIBC_TUNABLES=glibc.pthread.stack_cache_size=0. > > Eek, of course, sorry I should have seen that :/ > > I can't see a way to intercept points within the lifetime of the > thread id where tid is <=0, but I'll send an update if I do. You could avoid joining the thread t and loop until pthread_getcpuclockid fails, and then check that the error is ESRCH. Thanks, Florian
On 2024-11-22 06:43, Florian Weimer wrote: >> I can't see a way to intercept points within the lifetime of the >> thread id where tid is <=0, but I'll send an update if I do. > > You could avoid joining the thread t and loop until > pthread_getcpuclockid fails, and then check that the error is ESRCH. That would mean considering a timeout as success as well, wouldn't it? Or put in an upper limit to iterations. Sid
* Siddhesh Poyarekar: > On 2024-11-22 06:43, Florian Weimer wrote: >>> I can't see a way to intercept points within the lifetime of the >>> thread id where tid is <=0, but I'll send an update if I do. >> You could avoid joining the thread t and loop until >> pthread_getcpuclockid fails, and then check that the error is ESRCH. > > That would mean considering a timeout as success as well, wouldn't it? > Or put in an upper limit to iterations. I believe the kernel will reset the TID to zero once the thread exits. Thanks, Florian
On 2024-11-22 06:53, Florian Weimer wrote: > * Siddhesh Poyarekar: > >> On 2024-11-22 06:43, Florian Weimer wrote: >>>> I can't see a way to intercept points within the lifetime of the >>>> thread id where tid is <=0, but I'll send an update if I do. >>> You could avoid joining the thread t and loop until >>> pthread_getcpuclockid fails, and then check that the error is ESRCH. >> >> That would mean considering a timeout as success as well, wouldn't it? >> Or put in an upper limit to iterations. > > I believe the kernel will reset the TID to zero once the thread exits. Yes, but the main thread will need to win the race to see the TID before the thread descriptor is invalidated. In fact, I wonder if we can even ensure that the main thread sees into only a valid (but not freed) thread descriptor. If we can't ensure that, why go through the pain of the race anyway? Wouldn't it be easier to put in a comment in this test saying that it's technically invalid, but we're using internal knowledge to test a path that cannot otherwise be reasonably tested? thanks, Sid
* Siddhesh Poyarekar: > On 2024-11-22 06:53, Florian Weimer wrote: >> * Siddhesh Poyarekar: >> >>> On 2024-11-22 06:43, Florian Weimer wrote: >>>>> I can't see a way to intercept points within the lifetime of the >>>>> thread id where tid is <=0, but I'll send an update if I do. >>>> You could avoid joining the thread t and loop until >>>> pthread_getcpuclockid fails, and then check that the error is ESRCH. >>> >>> That would mean considering a timeout as success as well, wouldn't it? >>> Or put in an upper limit to iterations. >> I believe the kernel will reset the TID to zero once the thread >> exits. > > Yes, but the main thread will need to win the race to see the TID > before the thread descriptor is invalidated. The thread descriptor is invalidated on join only, not on thread exit (unless the thread is detached). So I don't think this is a problem. Thanks, Florian
On 2024-11-22 07:18, Florian Weimer wrote: >> Yes, but the main thread will need to win the race to see the TID >> before the thread descriptor is invalidated. > > The thread descriptor is invalidated on join only, not on thread exit > (unless the thread is detached). So I don't think this is a problem. OK, I've sent an updated test with the join call dropped. Thanks, Sid
diff --git a/nptl/Makefile b/nptl/Makefile index ceb91afafc..9faad1964c 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -317,6 +317,7 @@ tests = \ tst-pthread-attr-affinity-fail \ tst-pthread-attr-sigmask \ tst-pthread-defaultattr-free \ + tst-pthread-getcpuclockid-invalid \ tst-pthread-gdb-attach \ tst-pthread-gdb-attach-static \ tst-pthread-key1-static \ diff --git a/nptl/tst-pthread-getcpuclockid-invalid.c b/nptl/tst-pthread-getcpuclockid-invalid.c new file mode 100644 index 0000000000..f1720687a7 --- /dev/null +++ b/nptl/tst-pthread-getcpuclockid-invalid.c @@ -0,0 +1,47 @@ +/* Smoke test to verify that pthread_getcpuclockid fails with invalid thread + IDs. + Copyright the GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <assert.h> +#include <errno.h> +#include <pthread.h> +#include <time.h> + +#include <support/check.h> +#include <support/test-driver.h> +#include <support/xthread.h> + +void * +thr (void *in) +{ + return in; +} + +int +do_test (void) +{ + pthread_t t = xpthread_create (NULL, thr, NULL); + xpthread_join (t); + + clockid_t c; + TEST_COMPARE (pthread_getcpuclockid (t, &c), ESRCH); + + return 0; +} + +#include <support/test-driver.c>
Exercise the invalid thread descriptor case to ensure full test coverage for the function. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- nptl/Makefile | 1 + nptl/tst-pthread-getcpuclockid-invalid.c | 47 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 nptl/tst-pthread-getcpuclockid-invalid.c