This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] nptl: Fix invalid Systemtap probe in pthread_join [BZ #24211]
On 2/11/19 4:22 PM, Florian Weimer wrote:
> 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, 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.
OK for master.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 2019-02-11 Florian Weimer <fweimer@redhat.com>
>
> [BZ #24211]
> * nptl/pthread_join_common.c (__pthread_timedjoin_ex): Do not read
> pd->result after the thread descriptor has been freed.
>
> 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;
OK. Store result into pd_result local.
> if (__glibc_likely (result == 0))
> {
> /* We mark the thread as terminated and as joined. */
OK. In between the store and the access we have either not freed the stack,
or we have freed the stack but already read pd->result. The compiler might
generate two paths and optimize both, but can't optimize the read of global
memory past __free_tcb() which is in another CU.
> @@ -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;
OK. Micro-optimization.
>
> /* 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);
OK. Read of pd_result is always valid regardless of stack state.
>
> return result;
> }
>
--
Cheers,
Carlos.