Messages in this thread | | | Date | Thu, 13 Jan 2022 17:28:28 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v2] tracing: Add test for user space strings when filtering on string pointers |
| |
On Thu, 13 Jan 2022 22:11:35 +0000 David Laight <David.Laight@ACULAB.COM> wrote:
> > Too bad that the __user attribute is stripped during a normal compile. > > But couldn't we add the information whether a pointer belongs to user > > or kernel space in the trace event definition? For syscall tracing it's > > easy, because pointer types in SYSCALL_DEFINE() and friends are always > > userspace pointers? > > Also, when the __user pointer is saved it MUST be checked for being > a valid user pointer (eg with access_ok(ptr, 1).
It's rather hard to even know if a pointer is __user or not. It could be some random address in any event field.
> > You really do need to remember whether the pointer is user or kernel > when you save it. > > I also suspect that you need to check for contexts where 'current' > isn't really valid (eg any kind on interrupt) and ensure the user > reads aren't even attempted.
It's not going to crash, even if it is required. The strncpy_from_user/kernel_nofault() should detect any of that, right? Or are those functions not safe to call?
> The excuse of being 'root' in order to request/read trace isn't > really a very good one.
Not sure what you are getting at here? If you are worried about tracing reading anything, then disable it. There's a lockdown on tracing. for those that are worried.
Heck I could just do:
# echo 'p:random_umem __common_interrupt add=+0(@0x7f073a188000):u64' > /sys/kernel/tracing/kprobe_events # trace-cmd start -e random_umem # trace-cmd show # tracer: nop # # entries-in-buffer/entries-written: 15/15 #P:8 # # _-----=> irqs-off/BH-disabled # / _----=> need-resched # | / _---=> hardirq/softirq # || / _--=> preempt-depth # ||| / _-=> migrate-disable # |||| / delay # TASK-PID CPU# ||||| TIMESTAMP FUNCTION # | | | ||||| | | <idle>-0 [000] d.h1. 844.051612: random_umem: (__common_interrupt+0x0/0x100) add=7291432837672293 <idle>-0 [000] d.h1. 844.051694: random_umem: (__common_interrupt+0x0/0x100) add=74591483011526501 <idle>-0 [000] d.H1. 844.051743: random_umem: (__common_interrupt+0x0/0x100) add=21474836483 <idle>-0 [000] d.h1. 844.383333: random_umem: (__common_interrupt+0x0/0x100) add=115964116992 <idle>-0 [000] d.h1. 844.383802: random_umem: (__common_interrupt+0x0/0x100) add=1125968626319387 <idle>-0 [000] d.h1. 844.383864: random_umem: (__common_interrupt+0x0/0x100) add=115964116992 <idle>-0 [000] d.h1. 844.533321: random_umem: (__common_interrupt+0x0/0x100) add=4032068056083813 <idle>-0 [000] d.h1. 844.533801: random_umem: (__common_interrupt+0x0/0x100) add=6929886302 kworker/0:1-15 [000] d.h.. 844.900412: random_umem: (__common_interrupt+0x0/0x100) add=17367183 <idle>-0 [000] d.h1. 844.933428: random_umem: (__common_interrupt+0x0/0x100) add=115964116992 <idle>-0 [000] d.h1. 844.933818: random_umem: (__common_interrupt+0x0/0x100) add=17367183 <idle>-0 [000] d.h1. 844.933955: random_umem: (__common_interrupt+0x0/0x100) add=74591483008647173 <idle>-0 [000] d.h1. 845.364181: random_umem: (__common_interrupt+0x0/0x100) add=1125968626319387 bash-1759 [000] d.h.. 845.364541: random_umem: (__common_interrupt+0x0/0x100) add=13912554418208784 bash-1759 [000] d.h.. 845.364592: random_umem: (__common_interrupt+0x0/0x100) add=18446744071906760329
And at every interrupt I get the memory at: 0x7f073a188000 for the current running task that was running when the interrupt happened.
As I said. It's an API.
-- Steve
| |