relevant to PR25168: add user_string to bpf tapsets, x86_64 only

previously noted https://lwn.net/Articles/803587/

After some discussion with fche, I added user_string() but for x86_64 only.
After bpf_probe_read_user() helpers are added, can extend to other architectures.

* tapset/bpf/conversions.stp (kernel_string, kernel_string_n):
  note probe_read_str() future deprecation, fix error msg.
* tapset/uconversions.stp (user_string_n): add BPF variants.
(user_string_n_warn): add BPF variant.
* testsuite/systemtap.bpf/bpf.exp: launch /usr/bin/sleep to probe start of main().
* testsuite/systemtap.bpf/bpf_tests/user_string.stp: new testcase.

diff --git a/tapset/bpf/conversions.stp b/tapset/bpf/conversions.stp
index 26956fc135a9beccb43443095b20d7cacd2ce0bb..775429c183d8a0e7668d13693db42b98044fd267 100644 (file)
--- a/tapset/bpf/conversions.stp
+++ b/tapset/bpf/conversions.stp
@@ -37,6 +37,8 @@ function kernel_string:string (addr:long, err_msg:string)
      rc = bpf_probe_read_str(buf, BPF_MAXSTRINGLEN, addr); */
   alloc, $buf, BPF_MAXSTRINGLEN;
   0x62, $buf, -, -, 0x0; /* stw [$buf+0], 0x0 -- guarantee NUL byte */
+  /* TODO PR25168: probe_read_str is used for both kernel and user memory.
+     BPF will soon deprecate these in favour of separate functions. */
   call, $rc, probe_read_str, $buf, BPF_MAXSTRINGLEN, $addr;
 
   /* if (rc < 0) return err_msg;
@@ -76,11 +78,13 @@ function kernel_string_n:string (addr:long, n:long)
      rc = bpf_probe_read_str(buf, n, addr); */
   alloc, $buf, BPF_MAXSTRINGLEN;
   0x62, $buf, -, -, 0x0; /* stw [buf+0], 0 -- guarantee NUL byte */
+  /* TODO PR25168: probe_read_str is used for both kernel and user memory.
+     BPF will soon deprecate these in favour of separate functions. */
   call, $rc, probe_read_str, $buf, $n, $addr; /* TODO: should work if the helper is named bpf_probe_read_str() too */
 
   /* if (rc < 0) error("...", addr); */
   0x35, $rc, 0, _done, -; /* jge rc, 0, _done */
-  call, -, printf, "ERROR: kernel string copy fault at 0x%p [man error::fault]", $addr; /* TODO document stapbpf version of error::fault */
+  call, -, printf, "ERROR: string copy fault at 0x%p [man error::fault]", $addr; /* TODO document stapbpf version of error::fault */
   call, -, exit;
 
   label, _done;

diff --git a/tapset/macros.stpm b/tapset/macros.stpm
index 255cc0ca29480465a6300188a314cf37860c2f74..b9bf0c40b690774d773a190ea950ac28c87aa0a7 100644 (file)
--- a/tapset/macros.stpm
+++ b/tapset/macros.stpm
@@ -1,7 +1,8 @@
 // Return MAXSTRINGLEN
 @define MAXSTRINGLEN
 %(
-       @const("MAXSTRINGLEN")
+  // XXX: For BPF, keep in sync with BPF_MAXSTRINGLEN in bpf-internal.h.
+  %( runtime == "bpf" %? 64 %: @const("MAXSTRINGLEN") %)
 %)
 
 @define __compat_task

diff --git a/tapset/uconversions.stp b/tapset/uconversions.stp
index 06fb86b677e446c5308bca1cd61a0538e01df0e5..89ac55ec4e2c5c63d40657007977c5f8ff368de5 100644 (file)
--- a/tapset/uconversions.stp
+++ b/tapset/uconversions.stp
@@ -112,6 +112,7 @@ function user_string_quoted:string (addr:long) {
  * when userspace data is not accessible at the given address.
  */
 function user_string_n:string (addr:long, n:long)
+%( runtime != "bpf" %?
 %( systemtap_v < "2.3" %? // PR15044
        { return user_string_n(addr, n, "<unknown>") }
 %:
@@ -131,6 +132,12 @@ function user_string_n:string (addr:long, n:long)
                        STAP_RETVALUE[len - 1] = '\0';
        %}
 %)
+%:
+   { /* pure */ /* bpf */
+     // TODO: Does not provide address in error message.
+     return user_string_n(addr, n, "user string copy fault")
+   }
+%)
 
 /**
  * sfunction user_string_n - Retrieves string of given length from user space
@@ -145,6 +152,7 @@ function user_string_n:string (addr:long, n:long)
  * address.
  */
 function user_string_n:string (addr:long, n:long, err_msg:string)
+%( runtime != "bpf" %?
 %{ /* pure */ /* myproc-unprivileged */ /* unmodified-fnargs */
   int64_t len = clamp_t(int64_t, STAP_ARG_n + 1, 1, MAXSTRINGLEN - 1);
   if (_stp_strncpy_from_user(STAP_RETVALUE, 
@@ -153,6 +161,27 @@ function user_string_n:string (addr:long, n:long, err_msg:string)
   else
     STAP_RETVALUE[len - 1] = '\0';
 %}
+%:
+   { /* pure */ /* bpf */
+     /* !!! ACHTUNG !!!
+      * bpf uses the same bpf_probe_read() helpers for kernel and user
+      * addresses, on the assumption that the address spaces coincide.
+      * Which only really works on x86_64 in Current Day.
+      *
+      * If the address space is changed, it may return the wrong data.
+      * TODO PR25168: Fix this as soon as BPF ships proper, separate
+      * bpf_probe_read_{user,kernel}() helpers.
+      */
+     %( arch == "x86_64" %?
+        // TODO: Does not use the provided err_msg.
+        return kernel_string_n(addr, n) // calls probe_read_str()
+     %:
+        // TODO: Use error() function.
+        print("ERROR(unsupported): %s", err_msg)
+        exit()
+     %)
+   }
+%)
 function user_string_n2:string (addr:long, n:long, err_msg:string) {
   return user_string_n(addr, n, err_msg);
 }
@@ -189,6 +218,7 @@ function user_string_n_warn:string (addr:long, n:long) {
  * not abort) about the failure.
  */
 function user_string_n_warn:string (addr:long, n:long, warn_msg:string)
+%( runtime != "bpf" %?
 %{ /* pure */ /* myproc-unprivileged */ /* unmodified-fnargs */
        int64_t len = clamp_t(int64_t, STAP_ARG_n + 1, 1, MAXSTRINGLEN - 1);
        long rc;
@@ -205,6 +235,24 @@ function user_string_n_warn:string (addr:long, n:long, warn_msg:string)
        } else
                STAP_RETVALUE[len - 1] = '\0';
 %}
+%:
+   { /* pure */ /* bpf */
+     /* !!! ACHTUNG !!!
+      * bpf uses the same bpf_probe_read() helpers for kernel and user
+      * addresses, on the assumption that the address spaces coincide.
+      * Which only really works on x86_64 in Current Day.
+      *
+      * If the address space is changed, it may return the wrong data.
+      * TODO PR25168: Fix this as soon as BPF ships proper, separate
+      * bpf_probe_read_{user,kernel}() helpers.
+      */
+     %( arch == "x86_64" %?
+        return kernel_string_n(addr, n, warn_msg) // calls probe_read_str()
+     %:
+        return warn_msg // don't even bother
+     %)
+   }
+%)
 function user_string2_n_warn:string (addr:long, n:long, warn_msg:string) {
   user_string_n_warn(addr, n, warn_msg);
 }

diff --git a/testsuite/systemtap.bpf/bpf.exp b/testsuite/systemtap.bpf/bpf.exp
index 50c8c0e9c35d3536112a67e9adf16e507551cabc..2bec181c750f18b72ff23e138a653a339b4f9fdf 100644 (file)
--- a/testsuite/systemtap.bpf/bpf.exp
+++ b/testsuite/systemtap.bpf/bpf.exp
@@ -45,6 +45,9 @@ proc stapbpf_run { TEST_NAME OUTPUT_STR options args } {
     # don't the following: ... $test_file_name could be some transient or leftover file
     # if [file readable $test_file_name] { lappend cmd $test_file_name }
 
+    # child process, only for some commands
+    set mypid2 0
+
     send_log "executing: $cmd\n"
     eval spawn $cmd
     set mypid [exp_pid -i $spawn_id]
@@ -64,6 +67,13 @@ proc stapbpf_run { TEST_NAME OUTPUT_STR options args } {
             # Increase this to 8K worth of data.
             exp_match_max 8192
 
+            # create a child process for the tests that require one
+            switch $TEST_NAME {
+                user_string.stp {
+                    eval exec /usr/bin/sleep 2
+                }
+            }
+
             # Avoid PR17274 to propagate
             set origexpinternal 1
             if {"[exp_internal -info]" == "0"} {set origexpinternal 0}

diff --git a/testsuite/systemtap.bpf/bpf_tests/user_string.stp b/testsuite/systemtap.bpf/bpf_tests/user_string.stp
new file mode 100644 (file)
index 0000000..0565cf2
--- /dev/null
+++ b/testsuite/systemtap.bpf/bpf_tests/user_string.stp
@@ -0,0 +1,15 @@
+// note: requires coreutils-debuginfo
+
+probe begin {
+  printf("BEGIN\n")
+}
+
+probe process("/usr/bin/sleep").function("main") {
+  arg = user_string($argv[0])
+  printf("found %s\n", arg)
+  exit()
+}
+
+probe end {
+  printf("END PASS\n")
+}