i#5784: Mitigate drwrap retaddr transparency violation (#5896)

Adds a new drwrap API, drwrap_get_retaddr_if_sentinel(), that allows
mitigation of a transparency violation under the DRWRAP_REPLACE_RETADDR drwrap
strategy where the return address on the stack is replaced with the address of
the internal replace_retaddr_sentinel() routine. This API modifies the passed-in
value to the actual return address of the inner-most nested wrapped function if
the passed-in value is replace_retaddr_sentinel() itself.

Fixes the value of the marker written by the kernel xfer event in drmemtrace by
using the new drwrap_get_retaddr_if_sentinel() API on the mcontext PC before
writing it out to the trace. Before, this caused many invariant errors of type 'Signal
handler return point incorrect' in traces collected on proprietary apps.

Verified on a large proprietary app that this error due to drwrap is fixed now,
whereas there were a few hundred instances before.

Fixes: #5784

clients/drcachesim/tracer/tracer.cpp

@@ -1,5 +1,5 @@
 /* ******************************************************************************
- * Copyright (c) 2011-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2023 Google, Inc.  All rights reserved.
  * Copyright (c) 2010 Massachusetts Institute of Technology  All rights reserved.
  * ******************************************************************************/
 
@@ -1416,6 +1416,16 @@ event_kernel_xfer(void *drcontext, const dr_kernel_xfer_info_t *info)
      * for online though.
      */
     if (info->source_mcontext != nullptr) {
+        app_pc mcontext_pc = info->source_mcontext->pc;
+        /* When a signal arrives at the end of functions wrapped using the
+         * DRWRAP_REPLACE_RETADDR drwrap strategy, the mcontext PC on the stack
+         * is the address of the internal replace_retaddr_sentinel() instead
+         * of the actual return address of the wrapped function. For the
+         * kernel xfer marker to contain the correct value, we must handle
+         * this case by allowing drwrap to replace the address with the
+         * correct one.
+         */
+        drwrap_get_retaddr_if_sentinel(drcontext, &mcontext_pc);
         /* Enable post-processing to figure out the ordering of this xfer vs
          * non-memref instrs in the bb, and also to give core simulators the
          * interrupted PC -- primarily for a kernel event arriving right
@@ -1428,7 +1438,7 @@ event_kernel_xfer(void *drcontext, const dr_kernel_xfer_info_t *info)
              * the indexed segment.
              */
             uint64_t modoffs = reinterpret_cast<offline_instru_t *>(instru)->get_modoffs(
-                drcontext, info->source_mcontext->pc, &modidx);
+                drcontext, mcontext_pc, &modidx);
             /* We save space by using the modidx,modoffs format instead of a raw PC.
              * These 49 bits will always fit into the 48-bit value field unless the
              * module index is very large, when it will take two entries, while using
@@ -1447,9 +1457,9 @@ event_kernel_xfer(void *drcontext, const dr_kernel_xfer_info_t *info)
             marker_val = raw_pc.combined_value;
         } else
 #endif
-            marker_val = reinterpret_cast<uintptr_t>(info->source_mcontext->pc);
+            marker_val = reinterpret_cast<uintptr_t>(mcontext_pc);
         NOTIFY(3, "%s: source pc " PFX " => modoffs " PIFX "\n", __FUNCTION__,
-               info->source_mcontext->pc, marker_val);
+               mcontext_pc, marker_val);
     }
     if (info->type == DR_XFER_RSEQ_ABORT) {
         BUF_PTR(data->seg_base) += instru->append_marker(

ext/drwrap/drwrap.c

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2022 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2023 Google, Inc.  All rights reserved.
  * Copyright (c) 2008-2009 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -2771,6 +2771,19 @@ drwrap_get_stats(INOUT drwrap_stats_t *stats)
     return true;
 }
 
+DR_EXPORT
+void
+drwrap_get_retaddr_if_sentinel(void *drcontext, INOUT app_pc *possibly_sentinel)
+{
+    ASSERT(possibly_sentinel != NULL, "Input cannot be null.");
+    if ((app_pc)replace_retaddr_sentinel != *possibly_sentinel)
+        return;
+    per_thread_t *pt = (per_thread_t *)drmgr_get_tls_field(drcontext, tls_idx);
+    /* If we see the sentinel, we must be inside a wrapped function. */
+    ASSERT(pt != NULL && pt->wrap_level >= 0, "Invalid drwrap state.");
+    *possibly_sentinel = pt->retaddr[pt->wrap_level];
+}
+
 /***************************************************************************
  * Several different approaches to try and handle SEH/longjmp unwind
  */

ext/drwrap/drwrap.h

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2022 Google, Inc.   All rights reserved.
+ * Copyright (c) 2010-2023 Google, Inc.   All rights reserved.
  * **********************************************************/
 
 /* drwrap: DynamoRIO Function Wrapping and Replacing Extension
@@ -416,8 +416,10 @@ typedef enum {
      * However, this does violate transparency, and may cause some applications to fail.
      * In particular, detaching on AArchXX requires scanning the stack to find where the
      * return address was stored, which could conceivably replace an integer or
-     * non-pointer value that happens to match the sentinel used.  Use this at your own
-     * risk.
+     * non-pointer value that happens to match the sentinel used. Also, the transparency
+     * violation may be exposed to the client's dr_register_kernel_xfer_event() callback
+     * if it inspects the mcontext PC on the stack; drwrap_get_retaddr_if_sentinel()
+     * may be used to mitigate such cases. Use #DRWRAP_REPLACE_RETADDR at your own risk.
      */
     DRWRAP_REPLACE_RETADDR = 0x04,
 } drwrap_wrap_flags_t;
@@ -887,6 +889,18 @@ DR_EXPORT
 bool
 drwrap_get_stats(INOUT drwrap_stats_t *stats);
 
+DR_EXPORT
+/**
+ * If the provided app_pc (\p possibly_sentinel) is indeed the return address sentinel
+ * used to implement #DRWRAP_REPLACE_RETADDR, this routine replaces it with the actual
+ * return address of the inner-most nested wrapped function. Otherwise, it is a no-op.
+ * This allows mitigation of a transparency violation under the #DRWRAP_REPLACE_RETADDR
+ * strategy where the actual app return address on the stack is replaced with a return
+ * address sentinel.
+ */
+void
+drwrap_get_retaddr_if_sentinel(void *drcontext, INOUT app_pc *possibly_sentinel);
+
 /**@}*/ /* end doxygen group */
 
 #ifdef __cplusplus