public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
From: "dale.hamel at srvthe dot net" <sourceware-bugzilla@sourceware.org>
To: systemtap@sourceware.org
Subject: [Bug releng/25581] New: USDT probes when /proc/[pid]/mem not writeable
Date: Thu, 20 Feb 2020 02:00:00 -0000	[thread overview]
Message-ID: <bug-25581-6586@http.sourceware.org/bugzilla/> (raw)

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.

             reply	other threads:[~2020-02-20  2:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20  2:00 dale.hamel at srvthe dot net [this message]
2020-02-20  2:02 ` [Bug releng/25581] " dale.hamel at srvthe dot net
2020-02-20  2:03 ` dale.hamel at srvthe dot net
2020-02-20  2:06 ` dale.hamel at srvthe dot net
2020-02-20 22:55 ` dale.hamel at srvthe dot net
2020-02-21  0:18 ` dale.hamel at srvthe dot net
2020-02-21  3:35 ` dale.hamel at srvthe dot net
2020-02-21  3:35 ` dale.hamel at srvthe dot net
2020-02-21  3:48 ` dale.hamel at srvthe dot net
2020-02-21  6:08 ` dale.hamel at srvthe dot net
2020-02-21  6:38 ` dale.hamel at srvthe dot net
2020-02-21 15:46 ` dale.hamel at srvthe dot net
2020-02-21 19:02 ` dale.hamel at srvthe dot net

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-25581-6586@http.sourceware.org/bugzilla/ \
    --to=sourceware-bugzilla@sourceware.org \
    --cc=systemtap@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).