From 791ec491c372f49cea3ea7a7143454a9023ac9d4 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 17 Feb 2017 07:57:00 -0500 Subject: prlimit,security,selinux: add a security hook for prlimit When SELinux was first added to the kernel, a process could only get and set its own resource limits via getrlimit(2) and setrlimit(2), so no MAC checks were required for those operations, and thus no security hooks were defined for them. Later, SELinux introduced a hook for setlimit(2) with a check if the hard limit was being changed in order to be able to rely on the hard limit value as a safe reset point upon context transitions. Later on, when prlimit(2) was added to the kernel with the ability to get or set resource limits (hard or soft) of another process, LSM/SELinux was not updated other than to pass the target process to the setrlimit hook. This resulted in incomplete control over both getting and setting the resource limits of another process. Add a new security_task_prlimit() hook to the check_prlimit_permission() function to provide complete mediation. The hook is only called when acting on another task, and only if the existing DAC/capability checks would allow access. Pass flags down to the hook to indicate whether the prlimit(2) call will read, write, or both read and write the resource limits of the target process. The existing security_task_setrlimit() hook is left alone; it continues to serve a purpose in supporting the ability to make decisions based on the old and/or new resource limit values when setting limits. This is consistent with the DAC/capability logic, where check_prlimit_permission() performs generic DAC/capability checks for acting on another task, while do_prlimit() performs a capability check based on a comparison of the old and new resource limits. Fix the inline documentation for the hook to match the code. Implement the new hook for SELinux. For setting resource limits, we reuse the existing setrlimit permission. Note that this does overload the setrlimit permission to mean the ability to set the resource limit (soft or hard) of another process or the ability to change one's own hard limit. For getting resource limits, a new getrlimit permission is defined. This was not originally defined since getrlimit(2) could only be used to obtain a process' own limits. Signed-off-by: Stephen Smalley Signed-off-by: James Morris --- security/selinux/hooks.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0c2ac318aa7f..870d24ecc2de 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3920,6 +3920,19 @@ static int selinux_task_getioprio(struct task_struct *p) PROCESS__GETSCHED, NULL); } +int selinux_task_prlimit(const struct cred *cred, const struct cred *tcred, + unsigned int flags) +{ + u32 av = 0; + + if (flags & LSM_PRLIMIT_WRITE) + av |= PROCESS__SETRLIMIT; + if (flags & LSM_PRLIMIT_READ) + av |= PROCESS__GETRLIMIT; + return avc_has_perm(cred_sid(cred), cred_sid(tcred), + SECCLASS_PROCESS, av, NULL); +} + static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource, struct rlimit *new_rlim) { @@ -6206,6 +6219,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(task_setnice, selinux_task_setnice), LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio), LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio), + LSM_HOOK_INIT(task_prlimit, selinux_task_prlimit), LSM_HOOK_INIT(task_setrlimit, selinux_task_setrlimit), LSM_HOOK_INIT(task_setscheduler, selinux_task_setscheduler), LSM_HOOK_INIT(task_getscheduler, selinux_task_getscheduler), -- cgit v1.2.2 From 84e6885e9e6a818d1ca1eabb9b720b357ab07a8b Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 28 Feb 2017 09:35:08 -0500 Subject: selinux: fix kernel BUG on prlimit(..., NULL, NULL) commit 79bcf325e6b32b3c ("prlimit,security,selinux: add a security hook for prlimit") introduced a security hook for prlimit() and implemented it for SELinux. However, if prlimit() is called with NULL arguments for both the new limit and the old limit, then the hook is called with 0 for the read/write flags, since the prlimit() will neither read nor write the process' limits. This would in turn lead to calling avc_has_perm() with 0 for the requested permissions, which triggers a BUG_ON() in avc_has_perm_noaudit() since the kernel should never be invoking avc_has_perm() with no permissions. Fix this in the SELinux hook by returning immediately if the flags are 0. Arguably prlimit64() itself ought to return immediately if both old_rlim and new_rlim are NULL since it is effectively a no-op in that case. Reported by the lkp-robot based on trinity testing. Signed-off-by: Stephen Smalley Acked-by: Paul Moore Signed-off-by: James Morris --- security/selinux/hooks.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 870d24ecc2de..3ba5ce1f4e05 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3925,6 +3925,8 @@ int selinux_task_prlimit(const struct cred *cred, const struct cred *tcred, { u32 av = 0; + if (!flags) + return 0; if (flags & LSM_PRLIMIT_WRITE) av |= PROCESS__SETRLIMIT; if (flags & LSM_PRLIMIT_READ) -- cgit v1.2.2 From ca97d939db114c8d1619e10a3b82af8615372dae Mon Sep 17 00:00:00 2001 From: James Morris Date: Wed, 15 Feb 2017 00:18:51 +1100 Subject: security: mark LSM hooks as __ro_after_init Mark all of the registration hooks as __ro_after_init (via the __lsm_ro_after_init macro). Signed-off-by: James Morris Acked-by: Stephen Smalley Acked-by: Kees Cook --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3ba5ce1f4e05..d37a72316e9d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6123,7 +6123,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) #endif -static struct security_hook_list selinux_hooks[] = { +static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr), LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction), LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder), -- cgit v1.2.2 From e2f586bd83177d22072b275edd4b8b872daba924 Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Mon, 6 Mar 2017 19:46:14 +0100 Subject: selinux: check for address length in selinux_socket_bind() KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of uninitialized memory in selinux_socket_bind(): ================================================================== BUG: KMSAN: use of unitialized memory inter: 0 CPU: 3 PID: 1074 Comm: packet2 Tainted: G B 4.8.0-rc6+ #1916 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 0000000000000000 ffff8800882ffb08 ffffffff825759c8 ffff8800882ffa48 ffffffff818bf551 ffffffff85bab870 0000000000000092 ffffffff85bab550 0000000000000000 0000000000000092 00000000bb0009bb 0000000000000002 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x238/0x290 lib/dump_stack.c:51 [] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1008 [] __msan_warning+0x5b/0xb0 mm/kmsan/kmsan_instr.c:424 [] selinux_socket_bind+0xf41/0x1080 security/selinux/hooks.c:4288 [] security_socket_bind+0x1ec/0x240 security/security.c:1240 [] SYSC_bind+0x358/0x5f0 net/socket.c:1366 [] SyS_bind+0x82/0xa0 net/socket.c:1356 [] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [] entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.o:? chained origin: 00000000ba6009bb [] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67 [< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322 [< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337 [] kmsan_internal_chain_origin+0x118/0x1e0 mm/kmsan/kmsan.c:530 [] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380 [] SYSC_bind+0x129/0x5f0 net/socket.c:1356 [] SyS_bind+0x82/0xa0 net/socket.c:1356 [] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [] return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.o:? origin description: ----address@SYSC_bind (origin=00000000b8c00900) ================================================================== (the line numbers are relative to 4.8-rc6, but the bug persists upstream) , when I run the following program as root: ======================================================= #include #include #include int main(int argc, char *argv[]) { struct sockaddr addr; int size = 0; if (argc > 1) { size = atoi(argv[1]); } memset(&addr, 0, sizeof(addr)); int fd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); bind(fd, &addr, size); return 0; } ======================================================= (for different values of |size| other error reports are printed). This happens because bind() unconditionally copies |size| bytes of |addr| to the kernel, leaving the rest uninitialized. Then security_socket_bind() reads the IP address bytes, including the uninitialized ones, to determine the port, or e.g. pass them further to sel_netnode_find(), which uses them to calculate a hash. Signed-off-by: Alexander Potapenko Acked-by: Eric Dumazet [PM: fixed some whitespace damage] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d37a72316e9d..e67a526d1f30 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4367,10 +4367,18 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in u32 sid, node_perm; if (family == PF_INET) { + if (addrlen < sizeof(struct sockaddr_in)) { + err = -EINVAL; + goto out; + } addr4 = (struct sockaddr_in *)address; snum = ntohs(addr4->sin_port); addrp = (char *)&addr4->sin_addr.s_addr; } else { + if (addrlen < SIN6_LEN_RFC2133) { + err = -EINVAL; + goto out; + } addr6 = (struct sockaddr_in6 *)address; snum = ntohs(addr6->sin6_port); addrp = (char *)&addr6->sin6_addr.s6_addr; -- cgit v1.2.2