|
Message-ID: <CAG48ez1MdupASE1DMt0OWXv+rqZO-2GAbfDthEQcvNXrc2pTsQ@mail.gmail.com> Date: Fri, 27 Jul 2018 20:58:48 +0200 From: Jann Horn <jannh@...gle.com> To: salyzyn@...gle.com Cc: Steven Rostedt <rostedt@...dmis.org>, Nick Desaulniers <ndesaulniers@...gle.com>, Golden_Miller83@...tonmail.ch, Greg KH <greg@...ah.com>, Kees Cook <keescook@...gle.com>, salyzyn@...roid.com, kernel list <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...hat.com>, kernel-team@...roid.com, stable@...r.kernel.org, Kernel Hardening <kernel-hardening@...ts.openwall.com>, Jeffrey Vander Stoep <jeffv@...gle.com> Subject: Re: [PATCH] tracing: do not leak kernel addresses +cc jeffv On Fri, Jul 27, 2018 at 8:47 PM Jann Horn <jannh@...gle.com> wrote: > > On Fri, Jul 27, 2018 at 8:41 PM Mark Salyzyn <salyzyn@...gle.com> wrote: > > > > Any system can chose to change the permissions of a sysfs node, default, DAC (and MAC) is just layers of multi-level security (or lack thereof). As well intentioned as a default DAC is in the kernel, leaking kernel addresses is still an attack surface that we want to close tightly. > > > > For instance on Android: > > > > chmod 0755 /sys/kernel/debug/tracing > > > > is in the common init.rc file ... > > > > If DAC has been adjusted at runtime to permit access to the node, I would think that if the caller does not have all the credentials/capabilities, we do want the addresses to be abstracted by the kernel. > > If you adjust the access controls on debugfs to permit things that > aren't possible upstream, you may have to add new access controls to > ensure that that doesn't lead to security issues. And, in fact, you > did: > > walleye:/ # ls -laZ /sys/kernel/debug > total 0 > drwxr-xr-x 100 root root u:object_r:debugfs:s0 0 2018-07-27 18:08 . > drwxr-xr-x 19 root root u:object_r:sysfs:s0 0 1970-06-04 10:30 .. > [...] > drwxr-xr-x 6 root root u:object_r:debugfs_tracing:s0 0 > 1970-01-01 01:00 tracing > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 tsens > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 tzdbg > drwxr-xr-x 4 root root u:object_r:debugfs_ufs:s0 0 > 1970-01-01 01:00 ufshcd0 > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 usb > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 usb-pdphy > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 usb_diag > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 vmem > -r--r--r-- 1 root root u:object_r:debugfs:s0 0 > 1970-01-01 01:00 wakeup_sources > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 2018-07-27 18:07 wcd_spi > drwxr-xr-x 2 root root u:object_r:debugfs:s0 0 > 2018-07-27 18:07 wdsp0 > drwxr-xr-x 2 root root u:object_r:debugfs_wlan:s0 0 > 2018-07-27 18:07 wlan0 > drwxr-xr-x 3 root root u:object_r:debugfs:s0 0 > 2018-07-27 18:07 wlan_qdf > > Stuff in the debugfs mount is labeled as "debugfs", with a few > exceptions. And the SELinux policy locks down access to debugfs: > > public/domain.te:neverallow { domain -init -vendor_init -system_server > -dumpstate } debugfs:file no_rw_file_perms; And yes, if you check from an ADB shell, you can still access the mentioned file even on walleye: walleye:/ $ ls -lZ /sys/kernel/debug/tracing/printk_formats -r--r--r-- 1 root root u:object_r:debugfs_tracing:s0 0 1970-01-01 01:00 /sys/kernel/debug/tracing/printk_formats walleye:/ $ cat /sys/kernel/debug/tracing/printk_formats 0xffffff9c60ba04de : "Rescheduling interrupts" 0xffffff9c60ba04f6 : "Function call interrupts" 0xffffff9c60ba050f : "CPU stop interrupts" 0xffffff9c60ba0523 : "Timer broadcast interrupts" 0xffffff9c60ba053e : "IRQ work interrupts" 0xffffff9c60ba0552 : "CPU wakeup interrupts" 0xffffff9c60ba0568 : "CPU backtrace" 0xffffff9c619a6600 : "rcu_sched" 0xffffff9c619a6a40 : "rcu_bh" 0xffffff9c619a6ef8 : "rcu_preempt" But that's only because you're coming from "shell" context, and "shell" context has explicitly been granted access to files labeled as debugfs_tracing: # systrace support - allow atrace to run allow shell debugfs_tracing_debug:dir r_dir_perms; allow shell debugfs_tracing:dir r_dir_perms; allow shell debugfs_tracing:file rw_file_perms; allow shell debugfs_trace_marker:file getattr; allow shell atrace_exec:file rx_file_perms; Normal apps can't access it, AFAICS. If you think that the contents of this particular file should not be exposed, because you think that even someone with ADB access or in traceur_app context shouldn't be able to see those pointers, then you may wish to make a change to how you're labeling tracefs files.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.