[Bug releng/25581] New: USDT probes when /proc/[pid]/mem not writeable

["dale.hamel at srvthe dot net" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=25581

            Bug ID: 25581
           Summary: USDT probes when /proc/[pid]/mem not writeable
           Product: systemtap
           Version: unspecified
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: releng
          Assignee: systemtap at sourceware dot org
          Reporter: dale.hamel at srvthe dot net
  Target Milestone: ---

Created attachment 12303
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12303&action=edit
The full patch against sys/sdt.h and for the dtrace python header generating
script

USDT probes may not be accessible in environments where /proc/[pid]/mem is not
writeable. For instance, Container OS used by Google's GKE product forces this
to read-only.

In kernels later than 4.20, there is a kernel interface to increment the
semaphore counters for USDT probes by passing the offset to perf_event_open. In
kernels prior to this, if /proc/[pid]/mem is read-only, there is no userspace
way to support USDT probes, then the only means remaining is the kernel.

It is possible to provide an alternative implementation for enabled probes in
sys/sdt.h, however. I was able to modify the standard "systemtap-sdt-devel"
package with these patches:

```patch
From 191d9b7554859deab59d85b9ebe2f28878adfe7d Mon Sep 17 00:00:00 2001
From: Dale Hamel <dale.hamel@shopify.com>
Date: Wed, 19 Feb 2020 14:05:30 -0500
Subject: [PATCH] Modify systemtap-sdt-dev to use uprobes instead of semaphores

---
 systemtap-sdt-dev/sys/sdt.h   |  2 +-
 systemtap-sdt-dev/usdt/dtrace | 60 ++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/systemtap-sdt-dev/sys/sdt.h b/systemtap-sdt-dev/sys/sdt.h
index c2de2a9..7eec9bb 100644
--- a/systemtap-sdt-dev/sys/sdt.h
+++ b/systemtap-sdt-dev/sys/sdt.h
@@ -171,7 +171,7 @@ __extension__ extern unsigned long long __sdt_unsp;
 #endif

 #define _SDT_ASM_BODY(provider, name, pack_args, args)               \
-  _SDT_ASM_1(990:  _SDT_NOP)                         \
+  _SDT_ASM_1(.ifndef provider##_##name##_check\n provider##_##name##_check:\n
.endif\n990: _SDT_NOP) \
   _SDT_ASM_3(      .pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,"note") \
   _SDT_ASM_1(      .balign 4)                        \
   _SDT_ASM_3(      .4byte 992f-991f, 994f-993f, _SDT_NOTE_TYPE)          \
diff --git a/systemtap-sdt-dev/usdt/dtrace b/systemtap-sdt-dev/usdt/dtrace
index 981cce1..edfe004 100755
--- a/systemtap-sdt-dev/usdt/dtrace
+++ b/systemtap-sdt-dev/usdt/dtrace
@@ -40,30 +40,31 @@ except ImportError:
 class _HeaderCreator(object):
     def init_semaphores(self, fdesc):
         # dummy declaration just to make the object file non-empty
-        fdesc.write("/* Generated by the Systemtap dtrace wrapper */\n\n")
-        fdesc.write("static void __dtrace (void) __attribute__((unused));\n")
-        fdesc.write("static void __dtrace (void) {}\n")
+        #fdesc.write("/* Generated by the Systemtap dtrace wrapper */\n\n")
+        #fdesc.write("static void __dtrace (void) __attribute__((unused));\n")
+        #fdesc.write("static void __dtrace (void) {}\n")
         fdesc.write("\n#include <sys/sdt.h>\n\n")

     def init_probes(self, fdesc):
         fdesc.write("/* Generated by the Systemtap dtrace wrapper */\n\n")
-        fdesc.write("\n#define _SDT_HAS_SEMAPHORES 1\n\n")
-        fdesc.write("\n#define STAP_HAS_SEMAPHORES 1 /* deprecated */\n\n")
         fdesc.write("\n#include <sys/sdt.h>\n\n")
+        fdesc.write("#define HACK_UPROBE_ENABLED(provider, name) \
+                    ((*(char *) provider##_##name##_check) & 0x90) != 0x90\n")

     def add_semaphore(self, this_provider, this_probe):
         # NB: unsigned short is fixed in ABI
-        semaphores_def = '\n#if defined STAP_SDT_V1\n'
-        semaphores_def += '#define %s_%s_semaphore %s_semaphore\n' % \
-                          (this_provider, this_probe, this_probe)
-        semaphores_def += '#endif\n'
-        semaphores_def += '#if defined STAP_SDT_V1 || defined STAP_SDT_V2 \n'


```

Obviously if this were to be submitted upstream, a proper way to switch between
reference counting and uprobe based semaphores would make be preferred, this is
from my first prototype. I would think this could probably use the existing
HAS_SDT_SEMAPHORE logic

I've been able to verify this with bpftrace and bcc on a kernel without write
access to /proc/[pid]/mem. 

This approach works by anchoring the existing asm label 990: to a symbolic
label, making it the definition for the prototype that is added to the header
in place of the semaphore code.

If there is support for this approach, I would be happy to modify the above
patch into a more modular form to be upstreamed.

-- 
You are receiving this mail because:
You are the assignee for the bug.