Re: Failure in syscall.open probe

[David Smith <dsmith at redhat dot com>]

On Mon, Aug 28, 2017 at 1:28 AM, Arkady <arkady.miasnikov@gmail.com> wrote:
> This patch follows the API and fixes a bug - increment src/dst pointers

... patch deleted ...

Your patch looked fine, but I decided to go a slightly different
route. We had a function, called kderef_string_(), over in
runtime/linux/loc2c-runtime.h, which did everything we needed here,
but it was hardcoded to only work for kernel strings. I abstracted it
slightly and renamed it '_stp_deref_string_nofault()'. Now
kderef_string() and _stp_strncpy_from_user() both call
_stp_deref_string_nofault(). The advantage here is that we're using
the same function to read kernel and user strings.

Assuming after a testsuite run I don't see any issues, I'll check this
in tomorrow.

====
diff --git a/runtime/linux/copy.c b/runtime/linux/copy.c
index c5022b5e2..892700865 100644
--- a/runtime/linux/copy.c
+++ b/runtime/linux/copy.c
@@ -39,24 +39,10 @@
  * <i>count</i> bytes and returns <i>count</i>.
  */

-/* XXX: see also kread/uread in loc2c-runtime.h */
-static long _stp_strncpy_from_user(char *dst, const char __user *src,
long count)
+static long _stp_strncpy_from_user(char *dst, const char __user *src,
+   long count)
 {
- long res = -EFAULT;
-        mm_segment_t _oldfs = get_fs();
-        set_fs(USER_DS);
-        pagefault_disable();
-        /* XXX: The following preempt() manipulations should be
-           redundant with probe entry/exit code, but for unknown
-           reasons on RHEL5/6 conversions.exp intermittently fails
-           without this.  */
-        preempt_disable();
- if (!lookup_bad_addr(VERIFY_READ, (const unsigned long)src, count))
- res = strncpy_from_user(dst, src, count);
-        preempt_enable_no_resched();
-        pagefault_enable();
-        set_fs(_oldfs);
- return res;
+ return _stp_deref_string_nofault(dst, src, count, USER_DS);
 }

 /** Copy a block of data from user space.
diff --git a/runtime/linux/loc2c-runtime.h b/runtime/linux/loc2c-runtime.h
index 2920abed7..39dece07d 100644
--- a/runtime/linux/loc2c-runtime.h
+++ b/runtime/linux/loc2c-runtime.h
@@ -716,16 +716,30 @@ static inline char *kderef_buffer_(char *dst,
void *addr, size_t len)
     _r; \
   })

-/* The following is for kernel strings, see the uconversions.stp
-   tapset for user_string functions. */
+/*
+ * _stp_deref_string_nofault(): safely read a string from memory
+ * without a DEREF_FAULT on error
+ *
+ * dst: read the string into this address
+ * addr: address to read from
+ * len: maximum number of bytes to read
+ * seg: memory segment to use, either kernel (KERNEL_DS) or user
+ * (USER_DS)
+ *
+ * If this function gets an error while trying to read memory, -EFAULT
+ * is returned. On success, the number of bytes copied is returned
+ * (not including the trailing NULL). Note that the kernel will not
+ * pagefault when trying to read the string.
+ */

-static inline char *kderef_string_(char *dst, void *addr, size_t len)
+static inline long _stp_deref_string_nofault(char *dst, const char *addr,
+     size_t len, mm_segment_t seg)
 {
   int err = 0;
-  size_t i;
+  size_t i = 0;
   mm_segment_t oldfs = get_fs();

-  set_fs(KERNEL_DS);
+  set_fs(seg);
   pagefault_disable();
   if (lookup_bad_addr(VERIFY_READ, (uintptr_t)addr, len))
     err = 1;
@@ -746,13 +760,13 @@ static inline char *kderef_string_(char *dst,
void *addr, size_t len)
   pagefault_enable();
   set_fs(oldfs);

-  return err ? (char *)-1 : dst;
+  return err ? -EFAULT : i;
 }

 #define kderef_string(dst, addr, maxbytes) \
   ({ \
-    char *_r = kderef_string_((dst), (void *)(uintptr_t)(addr), (maxbytes)); \
-    if (_r == (char *)-1) \
+    long _r = _stp_deref_string_nofault((dst), (void
*)(uintptr_t)(addr), (maxbytes), KERNEL_DS); \
+    if (_r < 0) \
       DEREF_FAULT(addr); \
     _r; \
   })
====
-- 
David Smith
Principal Software Engineer
Red Hat