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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20220110122938.7b6a8847@gandalf.local.home>
Date:   Mon, 10 Jan 2022 12:29:38 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     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>
Subject: Re: [PATCH v2] tracing: Add test for user space strings when
 filtering on  string pointers

On Mon, 10 Jan 2022 12:24:36 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> > > Pingfan reported that the following causes a fault:
> > > 
> > >   echo "filename ~ \"cpu\"" > events/syscalls/sys_enter_openat/filter
> > >   echo 1 > events/syscalls/sys_enter_at/enable
> > > 
> > > The reason is that trace event filter treats the user space pointer
> > > defined by "filename" as a normal pointer to compare against the "cpu"
> > > string. If the string is not loaded into memory yet, it will trigger a
> > > fault in kernel space:    
> > 
> > If a userspace pointer can end up the kernel structure then presumably
> > a 'dodgy' user program can supply an arbitrary kernel address instead?
> > This may give the user the ability to read arbitrary kernel addresses
> > (including ones that are mapped to PCIe IO addresses).
> > Doesn't sound good at all.  
> 
> Only root has access to the information read here. All tracing requires
> root or those explicitly given access to the tracing data, which pretty
> much allows all access to kernel internals (including all memory). So
> nothing to worry about here ;-)

BTW, this patch doesn't give any new access to anything. It only protects
if the pointer is pointing someplace that will give a fault. The user could
today point that pointer to anything in kernel memory and trace it (but
only read it if you are root). And worse, if the pointer is to something
that can fault, it will trigger a BUG(). This could happen if root creates
the filter, and a non-privileged user sets that pointer to something bad.
Which means anyone can cause the machine to fault if root does this kind
of tracing. This is why I added the stable tag.

This patch is to prevent that BUG() from happening.

-- Steve