Bug 11444 - Check that no pt_regs*-processing functions are /* unprivileged */
Summary: Check that no pt_regs*-processing functions are /* unprivileged */
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: tapsets (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-29 18:49 UTC by Dave Brolley
Modified: 2011-01-11 19:23 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Brolley 2010-03-29 18:49:48 UTC 
... since uprobes pt_regs may leak kernel data
Comment 1 Frank Ch. Eigler 2010-06-11 17:51:09 UTC 
Reviewed all tapset functions.  Ones that
use CONTEXT->regs:

registers_valid() valid, only nullness test
user_mode() valid, only to test user mode flag in MSR
stp_pid() only mentioned for completeness; uses task_pt_regs() only

ucontext-unwind.stp:*: unsure, maybe should task_pt_regs() only; mjw?

utrace.stp: Ditto. Actually, should rewrite src/tapset-utrace.cxx
            to use expression embedded-c and remove these tapset fns entirely.
Comment 2 Mark Wielaard 2010-06-14 09:34:38 UTC 
(In reply to comment #1)
> ucontext-unwind.stp:*: unsure, maybe should task_pt_regs() only; mjw?

That comes from the fact that most backtracing code was written before user
space support was added. All code handles both in-kernel and in-user backtraces,
up to the dwarf unwinder. We could push the kernel/user split down into the
tapset functions and only pass around a pt_regs struct that is tailored to
kernel/user if that is safer. What exactly is the problem with passing around
CONTEXT->regs?
Comment 3 Mark Wielaard 2010-07-01 14:47:42 UTC 
(In reply to comment #2)
> What exactly is the problem with passing around CONTEXT->regs?

OK, I finally got a clue...

commit ec3c800b3556d90be2b2e810d2f0a496ddaba08b
Author: Mark Wielaard <mjw@redhat.com>
Date:   Thu Jul 1 16:42:30 2010 +0200

    PR11444 Don't leak CONTEXT->regs in unwind, explicitly use task_pt_regs.
    
    CONTEXT->regs might be kernel regs, or not set at all, so check whether
    we have a real task and grab the task_pt_regs before proceeding.
    
    * tapset/ucontext-unwind.stp (print_ubacktrace): Check current->mm
      and grab task_pt_regs.
      (print_ubacktrace_brief): Likewise.
      (ubacktrace): Likewise.
Comment 4 Dave Brolley 2011-01-11 19:23:14 UTC 
Problem fixed long ago ...