[Bug runtime/20236] New: code cleanup: simplify user/kernel memory access routines

["dsmith at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

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

            Bug ID: 20236
           Summary: code cleanup: simplify user/kernel memory access
                    routines
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: runtime
          Assignee: systemtap at sourceware dot org
          Reporter: dsmith at redhat dot com
  Target Milestone: ---

Right now we've got a multitude of routines to access user memory and kernel
memory. In addition, some of these routines call systemtap's
'lookup_bad_addr()' function, some don't. Some of them call the kernel's
'access_ok()' function, some don't. Note that lookup_bad_addr() doesn't call
access_ok(), which it probably should.

This should be cleaned up and hopefully simplified:

runtime/stp_string.h:

_stp_read_address(): calls access_ok(), then calls __stp_get_user()

runtime/stp_string.c: 

_stp_decode_utf8(): wrapper around _stp_read_address()
_stp_text_str(): wrapper around _stp_decode_utf8()

runtime/linux/copy.c:

__stp_strncpy_from_user(): does no address validity check
_stp_strncpy_from_user(): calls access_ok(), then calls __stp_strncpy_from_user
_stp_copy_from_user(): calls access_ok(), then calls
__copy_from_user_inatomic()

runtime/linux/loc2c-runtime.h:

_stp_deref(): calls lookup_bad_addr()
_stp_store_deref(): calls lookup_bad_addr()

(Note that kderef()/uderef()/kread()/uread() are just wrappers around
_stp_deref(). Similarly store_kderef()/store_uderef()/kwrite()/uwrite() are
just wrappers around _stp_store_deref().)

kderef_buffer()/kderef_string() are just wrappers around kderef(). However,
this means the address check is done 1 byte at a time. It might make more sense
to do one full address check at the start.

store_kderef_string() is a wrapper around store_kderef(). Like the previous
macros, this means the address check is done 1 byte at a time. It might make
more sense to do one full address check at the start.


Then there is tapset/uconversions.stp. It defines a macro, called
__STP_GET_USER(), that is a wrapper around uread(). However, it discards
errors.

This makes things a bit confusing, since the tapset function user_long(), which
calls __STP_GET_USER(), won't error on an invalid address. But, the tapset
function user_string(), which ends up calling _stp_strncpy_from_user(), will
error on an invalid address.

Sigh.

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