i#5784: Mitigate drwrap retaddr transparency violation

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_replace_if_retaddr_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_replace_if_retaddr_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 kernel xfer event callback if it
+     * inspects the mcontext PC on the stack; drwrap_replace_if_retaddr_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 address of the
+ * internal replace_retaddr_sentinel(), 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 return address on the stack is replaced with the address of the
+ * internal replace_retaddr_sentinel().
+ */
+void
+drwrap_replace_if_retaddr_sentinel(void *drcontext, INOUT app_pc *possibly_sentinel);
+
 /**@}*/ /* end doxygen group */
 
 #ifdef __cplusplus