| Message ID | 20241128113259.2113810-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | pthread_getcpuclockid: Add descriptive comment to smoke test | expand |
* Siddhesh Poyarekar: > +/* The input thread descriptor to pthread_getcpuclockid needs to be valid when > + the function is called. For the purposes of this test, this means that the > + thread should not be detached, have exited, but not joined. There is also a > + tiny window where the TCB has been allocated but the target thread itself > + not created, where pthread_getcpuclockid could return ESRCH, but it's not > + possible to tap into that window to reliably test it. It's not necessary > + anyway, since the window this test exploits is good enough to complete > + coverage for pthread_getcpuclockid alongside tst-clock2. */ The kernel stores the TID to the TCB before the system call returns (according to the clone manual page). So the only race is for thread exit, not thread creation, and the comment is misleading. Thanks, Florian
On 2024-11-28 07:00, Florian Weimer wrote: > * Siddhesh Poyarekar: > >> +/* The input thread descriptor to pthread_getcpuclockid needs to be valid when >> + the function is called. For the purposes of this test, this means that the >> + thread should not be detached, have exited, but not joined. There is also a >> + tiny window where the TCB has been allocated but the target thread itself >> + not created, where pthread_getcpuclockid could return ESRCH, but it's not >> + possible to tap into that window to reliably test it. It's not necessary >> + anyway, since the window this test exploits is good enough to complete >> + coverage for pthread_getcpuclockid alongside tst-clock2. */ > > The kernel stores the TID to the TCB before the system call returns > (according to the clone manual page). > > So the only race is for thread exit, not thread creation, and the > comment is misleading. I was thinking of a situation where the thread descriptor points to a TCB and the syscall has not been invoked yet; this would be exploitable when the thread descriptor itself is in memory shared with another thread. But now that I'm saying it out loud, it doesn't sound like behaviour that ought to be relied upon, just something that the internals currently happen to allow. How about this then: /* The input thread descriptor to pthread_getcpuclockid needs to be valid when the function is called. For the purposes of this test, this means that the thread should not be detached, have exited, but not joined. This should be good enough to complete coverage for pthread_getcpuclockid alongside tst-clock2. */ Thanks, Sid
* Siddhesh Poyarekar: > On 2024-11-28 07:00, Florian Weimer wrote: >> * Siddhesh Poyarekar: >> >>> +/* The input thread descriptor to pthread_getcpuclockid needs to be valid when >>> + the function is called. For the purposes of this test, this means that the >>> + thread should not be detached, have exited, but not joined. There is also a >>> + tiny window where the TCB has been allocated but the target thread itself >>> + not created, where pthread_getcpuclockid could return ESRCH, but it's not >>> + possible to tap into that window to reliably test it. It's not necessary >>> + anyway, since the window this test exploits is good enough to complete >>> + coverage for pthread_getcpuclockid alongside tst-clock2. */ >> The kernel stores the TID to the TCB before the system call returns >> (according to the clone manual page). >> So the only race is for thread exit, not thread creation, and the >> comment is misleading. > > I was thinking of a situation where the thread descriptor points to a > TCB and the syscall has not been invoked yet; this would be > exploitable when the thread descriptor itself is in memory shared with > another thread. But now that I'm saying it out loud, it doesn't sound > like behaviour that ought to be relied upon, just something that the > internals currently happen to allow. > > How about this then: > > /* The input thread descriptor to pthread_getcpuclockid needs to be > valid when > the function is called. For the purposes of this test, this means > that the > thread should not be detached, have exited, but not joined. This > should be > good enough to complete coverage for pthread_getcpuclockid alongside > tst-clock2. */ This looks reasonable to me (except for the line breaks). Thanks, Florian
diff --git a/nptl/TODO-testing b/nptl/TODO-testing index e076e5624f..f50d2ceb51 100644 --- a/nptl/TODO-testing +++ b/nptl/TODO-testing @@ -10,10 +10,6 @@ pthread_attr_[sg]etstack some more tests needed -pthread_getcpuclockid - - check that value is reset -> rt subdir - pthread_getschedparam pthread_setschedparam diff --git a/nptl/tst-pthread-getcpuclockid-invalid.c b/nptl/tst-pthread-getcpuclockid-invalid.c index e88a563427..df1ad18226 100644 --- a/nptl/tst-pthread-getcpuclockid-invalid.c +++ b/nptl/tst-pthread-getcpuclockid-invalid.c @@ -1,5 +1,4 @@ -/* Smoke test to verify that pthread_getcpuclockid fails with ESRCH when the - thread in question has exited. +/* pthread_getcpuclockid should fail with ESRCH when the thread exits. Copyright the GNU Toolchain Authors. This file is part of the GNU C Library. @@ -17,6 +16,15 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +/* The input thread descriptor to pthread_getcpuclockid needs to be valid when + the function is called. For the purposes of this test, this means that the + thread should not be detached, have exited, but not joined. There is also a + tiny window where the TCB has been allocated but the target thread itself + not created, where pthread_getcpuclockid could return ESRCH, but it's not + possible to tap into that window to reliably test it. It's not necessary + anyway, since the window this test exploits is good enough to complete + coverage for pthread_getcpuclockid alongside tst-clock2. */ + #include <errno.h> #include <pthread.h> #include <sched.h>