linux-kernel - Re: [PATCH v2] tracing: Add test for user space strings when filtering on string pointers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220113172828.54b46d7b@gandalf.local.home>
Date:   Thu, 13 Jan 2022 17:28:28 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     'Sven Schnelle' <svens@...ux.ibm.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Pingfan Liu <kernelfans@...il.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Tom Zanussi <zanussi@...nel.org>,
        "hca@...ux.ibm.com" <hca@...ux.ibm.com>,
        "deller@....de" <deller@....de>
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@...LAB.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