Tapset difficulties w/ pointers

Tapset difficulties w/ pointers

[Stone, Joshua I]

Hi all,

In working on the 'process' tapset, I've encountered a few difficulties
that I thought I would share, so we can search the collective mind for
solutions. I've split this into two emails to separate the related
parts...

For the process.exec probe, I would like to make the pointer to the new
task_struct available.  This is fine, and then I can make
access-functions for the user to extract data from this, like so:

  function task_pid:long (task:long) %{ /* pure */
      if (unlikely(THIS->task == 0))
          THIS->__retvalue = 0;
      else {
          struct task_struct *t = (struct task_struct
*)(long)THIS->task;
          THIS->__retvalue = t->tgid;
      }
  %}

The obvious problem here is safety.  I can check that the parameter is
not zero, but what do I do if the user calls task_pid(0xDEADBEEF)?
Oops...  Or what happens if the user stashes the pointer away in a
global, and tries to read it later?  Oops...

One solution is to "hide" task_pid(), perhaps by renaming it to
__task_pid(), and then have the tapset provide variables for pid, tid,
etc. and rely on the compiler to elide those that are unused.  Then the
pointer never needs to be given to the user.  This is still not really
safe though, because that __task_pid() is only discouraged, not
protected.  We probably could play a game with naming that only allows
calling some functions from within the same file, but these functions
would need to be duplicated multiple tapsets needs them.

A more general solution would be if we had some sort of "handle" type.
This might be an abstract data type that the script language is unable
to modify.  It can have metadata to identify the type, and perhaps also
to limit the valid lifetime of the pointer.  I haven't thought through
all of the implementation details, but I think that a handle type would
not be too far a stretch.

I welcome any feedback on this idea, and of course other ideas are
welcome as well.


Josh

Re: Tapset difficulties w/ pointers

[James Dickens]

On 4/28/06, Stone, Joshua I <joshua.i.stone@intel.com> wrote:
> Hi all,
>
> In working on the 'process' tapset, I've encountered a few difficulties
> that I thought I would share, so we can search the collective mind for
> solutions. I've split this into two emails to separate the related
> parts...
>
> For the process.exec probe, I would like to make the pointer to the new
> task_struct available.  This is fine, and then I can make
> access-functions for the user to extract data from this, like so:
>
>   function task_pid:long (task:long) %{ /* pure */
>       if (unlikely(THIS->task == 0))
>           THIS->__retvalue = 0;
>       else {
>           struct task_struct *t = (struct task_struct
> *)(long)THIS->task;
>           THIS->__retvalue = t->tgid;
>       }
>   %}
>
> The obvious problem here is safety.  I can check that the parameter is
> not zero, but what do I do if the user calls task_pid(0xDEADBEEF)?
> Oops...  Or what happens if the user stashes the pointer away in a
> global, and tries to read it later?  Oops...
>
simple answer, anytime you use a pointer systemtap's framework must be
prepared to fail, you have just scratched the surface of the way it
can fail, its not pretty but you have to catch all the error traps
while running your code and then do your best to back out any changes
your made and exit and leave the system in a runable state, if all
this can be done in a macro, that would be awesome else you will need
pointer access functions.

James Dickens
uadmin.blogspot.com



> One solution is to "hide" task_pid(), perhaps by renaming it to
> __task_pid(), and then have the tapset provide variables for pid, tid,
> etc. and rely on the compiler to elide those that are unused.  Then the
> pointer never needs to be given to the user.  This is still not really
> safe though, because that __task_pid() is only discouraged, not
> protected.  We probably could play a game with naming that only allows
> calling some functions from within the same file, but these functions
> would need to be duplicated multiple tapsets needs them.
>
> A more general solution would be if we had some sort of "handle" type.
> This might be an abstract data type that the script language is unable
> to modify.  It can have metadata to identify the type, and perhaps also
> to limit the valid lifetime of the pointer.  I haven't thought through
> all of the implementation details, but I think that a handle type would
> not be too far a stretch.
>
> I welcome any feedback on this idea, and of course other ideas are
> welcome as well.
>
>
> Josh
>

Re: Tapset difficulties w/ pointers

[Frank Ch. Eigler]

joshua.i.stone wrote:

> For the process.exec probe, I would like to make the pointer to the new
> task_struct available.  This is fine, and then I can make
> access-functions for the user to extract data from this, like so:
> 
>  function task_pid:long (task:long) %{ /* pure */
>  [...]
>    struct task_struct *t = [...] (THIS->task)-> [...]
>
> The obvious problem here is safety.  I can check that the parameter is
> not zero, but what do I do if the user calls task_pid(0xDEADBEEF)?
> Oops...  Or what happens if the user stashes the pointer away in a
> global, and tries to read it later?  Oops...

We don't have to return correct answers to trick questions, just avoid
hurting the system.  Dereference operations in exposed embedded-C code
can be channeled through the same deref() macro used by $target
variable expansions, which aims to catch ordinary faults.  The new
kprobes fault-handling code being discussed may do this automatically
for some probes.  Further, we can mark mapped-but-unsafe memory
regions (bug #1288) in a similar way as dtrace does, to filter
addresses further.

Some of this would require some discipline from the author of an
embedded-C function.


> One solution is to "hide" task_pid(), perhaps by renaming it to
> __task_pid(), and then have the tapset provide variables for pid,
> tid, etc. and rely on the compiler to elide those that are unused.
> [...]

This style should work well with recent optimizer tweaks.

> This is still not really safe though, because that __task_pid() is
> only discouraged, not protected.  [...]

Another possibility is to impose a naming convention for "internal"
functions (and probes? variables?), so that only guru-mode code can
resolve references to them.  (i.e., external references to "__FOO"
symbols would only resolve in guru mode).


> A more general solution would be if we had some sort of "handle"
> type.  [...]  It can have metadata to identify the type, and perhaps
> also to limit the valid lifetime of the pointer. [...]

Unless the machinery involved in something like this is fairly small
and simple, I'd prefer to first explore the limits of the simpler ones
above.


- FChE