Re: [PATCH] nptl: Fix invalid Systemtap probe in pthread_join [BZ #24211]

[Florian Weimer <fweimer at redhat dot com>]

* Andreas Schwab:

> On Feb 11 2019, Florian Weimer <fweimer@redhat.com> wrote:
>
>> diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
>> index ecb78ffba5..45deba6a74 100644
>> --- a/nptl/pthread_join_common.c
>> +++ b/nptl/pthread_join_common.c
>> @@ -101,7 +101,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, result);
>
> result and pd->result are not the same thing.

Oops.  What about this?

Or should I write essentially the same probe twice?  I don't want to
introduce a new probe name because I want to backport this (along with
the fix for bug 24164).

Thanks,
Florian

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.

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;
   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;
 }