Messages in this thread | | | Subject | Re: Does uaccess_kernel() work for detecting kernel thread? | From | Tetsuo Handa <> | Date | Tue, 5 Jan 2021 19:11:03 +0900 |
| |
On 2021/01/05 16:59, Christoph Hellwig wrote: > On Wed, Dec 23, 2020 at 07:11:38PM +0900, Tetsuo Handa wrote: >> due to commit 5e6e9852d6f76e01 ("uaccess: add infrastructure for kernel >> builds with set_fs()") and follow up changes. Don't we need to change this >> "uaccess_kernel()" with "(current->flags & PF_KTHREAD)" ? > > No. The real problem here is that when a this funtion is called
Called by "who" ? Called by "a userspace process" ? Called by "a kernel thread" ? Called by "an io_uring service thread" ?
> under > set_fs it allows kernel memory access for all user pointers, and due to > the indirection in the playload allows reading or changing kernel > memory. A kthread does not have that issue.
If this uaccess_kernel() is intended to reject calling this function from "a userspace process", uaccess_kernel() is failing to reject because uaccess_kernel() is always "false" for x86.
If this uaccess_kernel() is intended to reject calling this function from "a kernel thread", uaccess_kernel() is failing to reject because uaccess_kernel() is always "false" for x86.
If this uaccess_kernel() is intended to reject calling this function from "an io_uring service thread", uaccess_kernel() is failing to reject because uaccess_kernel() is always "false" for x86.
What does uaccess_kernel() in sg_check_file_access() (and uhid_char_write(), ib_safe_file_access(), bpfilter_process_sockopt() etc.) want to check?
> >>>> For another example, if uaccess_kernel() is "false" due to CONFIG_SET_FS=n, >>>> isn't TOMOYO unexpectedly checking permissions for socket operations? >>> >>> Can someone explain WTF TOMOYO is even doing there? A security module >>> has absolutely no business checking what context it is called from, but >>> must check the process credentials instead. >>> >> >> TOMOYO distinguishes userspace processes and kernel threads, and grants >> kernel threads implicit permissions to perform socket operations. > > And this is the problem we need to fix. A kernel thread can't just have > implicit permissions only because it is a kernel thread. Think of e.g. > the io_uring service threads.
We can use (current->flags & (PF_KTHREAD | PF_IO_WORKER)) == PF_KTHREAD like https://lkml.kernel.org/r/dacfb329-de66-d0cf-dcf9-f030ea1370de@schaufler-ca.com does in order to exclude e.g. the io_uring service threads, can't we?
| |