[Bug runtime/9937] New: verify user-space build-ids

[Bug runtime/9937] New: verify user-space build-ids

[fche at redhat dot com]

Upon the appropriate callbacks from task_finder, we should verify that
user-space _stp_modules have a matching build-id.  At this point, we have the
info available but only verify it for kernel/modules.

-- 
           Summary: verify user-space build-ids
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: runtime
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: fche at redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-03-13 02:28 -------
One problem is to find the matching "module" in _stp_modules array, available
info is name, build-ids, etc. But what we can have in callback from task_finder
is task_struct, stap_task_finder_target. One possible option is to compare
tsk->comm and module->name. But seems not exactly correct.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[mjw at redhat dot com]

------- Additional Comments From mjw at redhat dot com  2009-03-13 12:13 -------
On the pr6866 branch I have some changes to task_finder.c that matches and
attaches a module to a vma entry. That is probably the place to hook in the
build-id check. I'll port it to the trunk master branch asap.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[mjw at redhat dot com]

------- Additional Comments From mjw at redhat dot com  2009-03-15 19:03 -------
(In reply to comment #2)
> On the pr6866 branch I have some changes to task_finder.c that matches and
> attaches a module to a vma entry. That is probably the place to hook in the
> build-id check. I'll port it to the trunk master branch asap.

I did this:

commit bb64f40b58a64a9ae065dba5058463ac604c3896
Author: Mark Wielaard <mjw@redhat.com>
Date:   Sun Mar 15 15:29:01 2009 +0100

    Move vma module tracking from pr6866 branch to master.
    
    * tapsets.cxx (utrace_derived_probe_group::emit_module_decls):
      Always emit vm callback probe for __stp_tf_vm_cb.
    * runtime/task_finder.c (__stp_tf_vm_cb): Always expose, move _stp_dbug
      statements under ifdef DEBUG_TASK_FINDER_VMA. Find  and record
      corresponding module when vm_path not NULL.
    * runtime/task_finder_vma.c (struct __stp_tf_vma_entry): Add _stp_module.
      (stap_add_vma_map_info): Add _stp_module argument and assign.
      (__stp_tf_get_vma_entry_addr): New static function to get
      the __stp_tf_vma_entry given an address.

This only hooks for utrace, need to merge this method with the uprobes based
task_finder callbacks. Plus we need to make sure to always canonicalize the
module paths.

I haven't actually looked yet into how to fetch the build-id itself from the vma
and match it to what we stored in the module.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-03-16 06:32 -------
Created an attachment (id=3826)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=3826&action=view)
Draft code for checking build-id

This is the draft code for validating build-id. There are several iterms needed
to 
be confirmed.
1. Where the checking should happen,  especially after vma module tracking code
is visible by Mark.  Currently,  I put the check in stap_uprobe_process_found
or stap_uprobe_vmchange_found
2. Searching the found task in _stp_modules through tgt->pathname/vm_path.  Is
it enough?
3. Once build-id validation failed,  just return error? Any more cleanup need
to be done?
4. More consideration?


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[mjw at redhat dot com]

------- Additional Comments From mjw at redhat dot com  2009-03-16 21:45 -------
(In reply to comment #4)
> 1. Where the checking should happen,  especially after vma module tracking code
> is visible by Mark.  Currently,  I put the check in stap_uprobe_process_found
> or stap_uprobe_vmchange_found

Yes, this is where I would also hook in for tracking the vma->module map. I
don't think you need to hook stap_uprobe_process_found explicitly for an EXEC
finder.callback since the finder.vm_callback will give you also matches for the
EXEC itself. But maybe I am missing a timing issue.

> 2. Searching the found task in _stp_modules through tgt->pathname/vm_path.  Is
> it enough?

I believe this is enough, the vm_path is the complete canonical path. We do need
to make sure to register the module names also with their full canonical names.

> 3. Once build-id validation failed,  just return error? Any more cleanup need
> to be done?

I would only produce a warning, we are also just continuing when we cannot
verify the build-id. Or, if we want to make that also an error then we probably
have to do what stap_uprobe_change() at the end when things fail, set the state
to error and _stp_exit().

> 4. More consideration?

I am not sure about limiting the build-id comparison to MAXSTRINGLEN. If we are
afraid of people creating scripts against modules with absurdly big build-ids
then we should block those in the translator imho.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[mjw at redhat dot com]

------- Additional Comments From mjw at redhat dot com  2009-03-17 13:22 -------
(In reply to comment #5)
> > 2. Searching the found task in _stp_modules through tgt->pathname/vm_path.  Is
> > it enough?
> 
> I believe this is enough, the vm_path is the complete canonical path. We do need
> to make sure to register the module names also with their full canonical names.

I made sure they are through the following commit. The path is now used in the
taskfinder vma tracker to compare against the vm_path. Maybe name could be
completely replaced by path, but for the kernel and modules name is kind of
special (we do check for the "kernel" name in a couple of places). So for now it
is just an extra field for use at runtime.

commit 30cb532a560ed152b86506b80490e99195970271
Author: Mark Wielaard <mjw@redhat.com>
Date:   Tue Mar 17 13:50:33 2009 +0100

    Get the canonical path of the main file for comparison at runtime.
    
    When given directly by the user through -d or in case of the kernel
    name and path might differ. path should be used for matching.
    
    * runtime/sym.h (_stp_module): Add path field.
    * runtime/task_finder.c (__stp_tf_vm_cb): Use module path to compare vm_path
    * translate.cxx (dump_unwindsyms): Output canonical path.


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-03-27 03:33 -------
Created an attachment (id=3850)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=3850&action=view)
Updated patch

Changed to the old one.
1. Once build-id check finished, early return
2. Trigger warning if build-id doesn't match and make finder.callback return.
3. build-id checking limited to map_p in vma.callback
4. Use module path to compare vm_path. (thanks to Mark Wielaard)

Did some tests on FC10. Looks fine.

Need to be improve:
1. Still need to hook stap_uprobe_process_found explicitly for an EXEC
finder.callback since vma.callback == NULL.
2. The annoying problem is that the validation procedure will be repeated 
many times. That depends on callbacks. 

Any idea? 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #3826 is|0                           |1
           obsolete|                            |


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2009-03-27 11:03 -------
(In reply to comment #7)
> 1. Still need to hook stap_uprobe_process_found explicitly for an EXEC
> finder.callback since vma.callback == NULL.

Probably a task-finder change is needed.

> 2. The annoying problem is that the validation procedure will be repeated 
> many times. That depends on callbacks. 

Can you explain the nature of the repetition?


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-03-27 13:01 -------
(In reply to comment #8)
> (In reply to comment #7)
> > 1. Still need to hook stap_uprobe_process_found explicitly for an EXEC
> > finder.callback since vma.callback == NULL.
> 
> Probably a task-finder change is needed.
> 
> > 2. The annoying problem is that the validation procedure will be repeated 
> > many times. That depends on callbacks. 
> 
> Can you explain the nature of the repetition?
> 
Sorry, maybe I havn't expressed it clearly. I mean that current code can't 
ensure checking build-id only once. Once the callback is invoked, checking
procedure may be called too, especially process("foo").function("*") or vma
splitted.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[dsmith at redhat dot com]

------- Additional Comments From dsmith at redhat dot com  2009-03-27 14:07 -------
(In reply to comment #7)
> Need to be improve:
> 1. Still need to hook stap_uprobe_process_found explicitly for an EXEC
> finder.callback since vma.callback == NULL.

Can you explain this one a bit further?  Isn't stap_uprobe_process_found()
already getting called on exec events?

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-03-28 01:50 -------
(In reply to comment #10)
> (In reply to comment #7)
> > Need to be improve:
> > 1. Still need to hook stap_uprobe_process_found explicitly for an EXEC
> > finder.callback since vma.callback == NULL.
> 
> Can you explain this one a bit further?  Isn't stap_uprobe_process_found()
> already getting called on exec events?
stap_uprobe_process_found is OK. Please see comment #5.


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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

[Bug runtime/9937] verify user-space build-ids

[wenji dot huang at oracle dot com]

------- Additional Comments From wenji dot huang at oracle dot com  2009-04-13 07:52 -------
Created an attachment (id=3882)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=3882&action=view)
Updated patch -v3

 Guess the code now can be matured since related part PR9940 is resolved. 
Also, the flag
note_sect in stp_modules will ensure checking build-id only once. 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #3850 is|0                           |1
           obsolete|                            |


http://sourceware.org/bugzilla/show_bug.cgi?id=9937

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