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

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

[mjw at redhat dot com]

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

Mark Wielaard <mjw at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjw at redhat dot com

--- Comment #13 from Mark Wielaard <mjw at redhat dot com> 2010-10-05 17:45:22 UTC ---
The code has been slightly reworked. The code shouldn't have to add its own
hooks, but can be called from _stp_vma_mmap_cb (runtime/vma.c).

Currently this code only checks the file name paths match:

    if (strcmp(path, _stp_modules[i]->path) == 0)

That check can be augmented, or replaced by the build-id check.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[mjw at redhat dot com]

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

--- Comment #14 from Mark Wielaard <mjw at redhat dot com> 2010-10-05 17:55:00 UTC ---
Also note that there is a simple build-id check just for the vdso hooked from
_stp_vma_exec_cb in _stp_vma_match_vdso (runtime/vma.c). You might be able to
reuse some of this with the vdso_addr replaced with the addr given in the
_stp_vma_mmap_cb hook.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[fche at redhat dot com]

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

--- Comment #15 from Frank Ch. Eigler <fche at redhat dot com> 2010-10-05 17:59:02 UTC ---
Note that the translator-side build-id address calculation logic
may need to switch to emitting proper relocation bases (a la
the _stp_relocate_address section-name argument) for the
ET_DYN / ET_EXEC user-space cases, instead of storing a
single little offset.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[scox at redhat dot com]

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

--- Comment #16 from Stan Cox <scox at redhat dot com> 2010-10-20 18:13:59 UTC ---
Created attachment 5074
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5074
9937 patch

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[scox at redhat dot com]

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

Stan Cox <scox at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |scox at redhat dot com

--- Comment #17 from Stan Cox <scox at redhat dot com> 2010-10-20 18:15:27 UTC ---
(In reply to comment #16)

Add build id support for user modules.  The build id is an offset which is
adjusted relative to dwarf_module_base for ET_EXEC or the load address from the
task manager for ET_DYN.

    * sym.c (_stp_build_id_check): New.  Moved from... (plus addition
    of user_module parameter)
    (_stp_build_id_check): ...here.
    (_stp_userprog_check): New.  Build id check for user modules.
    * sym.h (dwarf_module_base): New.
    * uprobes-common.c (stap_uprobe_change_plus): Call _stp_userprog_check.
    * translate.cxx (dump_unwindsyms): Don't stop if it isn't a kernel
    module.
    Initialize module->dwarf_module_base.  Output module->build_id_offset
    for .dynamic case.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[scox at redhat dot com]

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

--- Comment #18 from Stan Cox <scox at redhat dot com> 2010-10-21 20:16:57 UTC ---
(uprobes*prelink is the only regression with the above patch)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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

[fche at redhat dot com]

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

Frank Ch. Eigler <fche at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|systemtap at sources dot    |scox at redhat dot com
                   |redhat.com                  |

--- Comment #19 from Frank Ch. Eigler <fche at redhat dot com> 2010-10-23 15:29:39 UTC ---
Looks like a good start.
A few problems:

The probe_kernel_* alternative is based on an autoconf macro
to determine the presence of the probe_kernel_* functions,
*not* whether we're trying to check a kernel- or user-space
build-id string.

I don't understand why the dwfl_module_getdwarf / dwarf_module_base
stuff is at all relevant here.  The build-id is not debuginfo
based.  It's as if you're adding then subtracting the same dwbias
value, to no effect at all.

Instead, the code should reuse/extend the explicit
dwfl_module_relocation_info* calculations as used for
kernel modules up near line 4895, which I see it's trying
to do (though the comments need to be expanded to explain).
It needs to respect/store the secname instead of just bypassing
the .note.gnu.build-id assertion (which is correct only for
kernel modules).

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[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.