Messages in this thread | | | Subject | Re: Does uaccess_kernel() work for detecting kernel thread? | From | Tetsuo Handa <> | Date | Wed, 23 Dec 2020 19:11:38 +0900 |
| |
On 2020/12/23 16:53, Christoph Hellwig wrote: > On Tue, Dec 22, 2020 at 11:39:08PM +0900, Tetsuo Handa wrote: >> For example, if uaccess_kernel() is "false" due to CONFIG_SET_FS=n, >> isn't sg_check_file_access() failing to detect kernel context? > > sg_check_file_access does exactly the right thing - fail for all kernel > threads as those can't support the magic it does.
My question is, in Linux 5.10, sg_check_file_access() for x86 became
static int sg_check_file_access(struct file *filp, const char *caller) { if (filp->f_cred != current_real_cred()) { pr_err_once("%s: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", caller, task_tgid_vnr(current), current->comm); return -EPERM; } if (0) { pr_err_once("%s: process %d (%s) called from kernel context, this is not allowed.\n", caller, task_tgid_vnr(current), current->comm); return -EACCES; } return 0; }
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)" ?
> >> 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. Since "uaccess_kernel()" became "0" for x86, TOMOYO is no longer able to grant kernel threads implicit permissions to perform socket operations. Since Eric says "For PF_IO_WORKER kernel threads which are running code on behalf of a user we want to perform the ordinary permission checks.", I think that TOMOYO wants to use "(current->flags & (PF_KTHREAD | PF_IO_WORKER)) == PF_KTHREAD" instead.
| |