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 ++++++++++++++ security/selinux/include/classmap.h | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'security/selinux') 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), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index d429c4a1c551..1e0cc9b5de20 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -47,7 +47,7 @@ struct security_class_mapping secclass_map[] = { "getattr", "setexec", "setfscreate", "noatsecure", "siginh", "setrlimit", "rlimitinh", "dyntransition", "setcurrent", "execmem", "execstack", "execheap", "setkeycreate", - "setsockcreate", NULL } }, + "setsockcreate", "getrlimit", NULL } }, { "system", { "ipc_info", "syslog_read", "syslog_mod", "syslog_console", "module_request", "module_load", NULL } }, -- cgit v1.2.3 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') 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.3 From dd0859dccbe291cf8179a96390f5c0e45cb9af1d Mon Sep 17 00:00:00 2001 From: James Morris Date: Wed, 15 Feb 2017 00:17:24 +1100 Subject: security: introduce CONFIG_SECURITY_WRITABLE_HOOKS Subsequent patches will add RO hardening to LSM hooks, however, SELinux still needs to be able to perform runtime disablement after init to handle architectures where init-time disablement via boot parameters is not feasible. Introduce a new kernel configuration parameter CONFIG_SECURITY_WRITABLE_HOOKS, and a helper macro __lsm_ro_after_init, to handle this case. Signed-off-by: James Morris Acked-by: Stephen Smalley Acked-by: Casey Schaufler Acked-by: Kees Cook --- include/linux/lsm_hooks.h | 7 +++++++ security/Kconfig | 5 +++++ security/selinux/Kconfig | 6 ++++++ 3 files changed, 18 insertions(+) (limited to 'security/selinux') diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index ba3049f05aea..1aa63335de9e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1920,6 +1920,13 @@ static inline void security_delete_hooks(struct security_hook_list *hooks, } #endif /* CONFIG_SECURITY_SELINUX_DISABLE */ +/* Currently required to handle SELinux runtime hook disable. */ +#ifdef CONFIG_SECURITY_WRITABLE_HOOKS +#define __lsm_ro_after_init +#else +#define __lsm_ro_after_init __ro_after_init +#endif /* CONFIG_SECURITY_WRITABLE_HOOKS */ + extern int __init security_module_enable(const char *module); extern void __init capability_add_hooks(void); #ifdef CONFIG_SECURITY_YAMA diff --git a/security/Kconfig b/security/Kconfig index d900f47eaa68..3ff1bf91080e 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -31,6 +31,11 @@ config SECURITY If you are unsure how to answer this question, answer N. +config SECURITY_WRITABLE_HOOKS + depends on SECURITY + bool + default n + config SECURITYFS bool "Enable the securityfs filesystem" help diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig index ea7e3efbe0f7..8af7a690eb40 100644 --- a/security/selinux/Kconfig +++ b/security/selinux/Kconfig @@ -40,6 +40,7 @@ config SECURITY_SELINUX_BOOTPARAM_VALUE config SECURITY_SELINUX_DISABLE bool "NSA SELinux runtime disable" depends on SECURITY_SELINUX + select SECURITY_WRITABLE_HOOKS default n help This option enables writing to a selinuxfs node 'disable', which @@ -50,6 +51,11 @@ config SECURITY_SELINUX_DISABLE portability across platforms where boot parameters are difficult to employ. + NOTE: selecting this option will disable the '__ro_after_init' + kernel hardening feature for security hooks. Please consider + using the selinux=0 boot parameter instead of enabling this + option. + If you are unsure how to answer this question, answer N. config SECURITY_SELINUX_DEVELOP -- cgit v1.2.3 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/apparmor/lsm.c | 2 +- security/commoncap.c | 2 +- security/loadpin/loadpin.c | 2 +- security/security.c | 2 +- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- security/tomoyo/tomoyo.c | 2 +- security/yama/yama_lsm.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) (limited to 'security/selinux') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 709eacd23909..e287b691a30e 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -587,7 +587,7 @@ static int apparmor_task_setrlimit(struct task_struct *task, return error; } -static struct security_hook_list apparmor_hooks[] = { +static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme), LSM_HOOK_INIT(capget, apparmor_capget), diff --git a/security/commoncap.c b/security/commoncap.c index 78b37838a2d3..7abebd782d5e 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -1071,7 +1071,7 @@ int cap_mmap_file(struct file *file, unsigned long reqprot, #ifdef CONFIG_SECURITY -struct security_hook_list capability_hooks[] = { +struct security_hook_list capability_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(capable, cap_capable), LSM_HOOK_INIT(settime, cap_settime), LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check), diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c index 1d82eae3a5b8..dbe6efde77a0 100644 --- a/security/loadpin/loadpin.c +++ b/security/loadpin/loadpin.c @@ -174,7 +174,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id) return 0; } -static struct security_hook_list loadpin_hooks[] = { +static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security), LSM_HOOK_INIT(kernel_read_file, loadpin_read_file), }; diff --git a/security/security.c b/security/security.c index 905dad2811d3..d6d18a3721aa 100644 --- a/security/security.c +++ b/security/security.c @@ -1628,7 +1628,7 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule, } #endif /* CONFIG_AUDIT */ -struct security_hook_heads security_hook_heads = { +struct security_hook_heads security_hook_heads __lsm_ro_after_init = { .binder_set_context_mgr = LIST_HEAD_INIT(security_hook_heads.binder_set_context_mgr), .binder_transaction = 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), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index fc8fb31fc24f..927e60e622d1 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4633,7 +4633,7 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) return 0; } -static struct security_hook_list smack_hooks[] = { +static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme), LSM_HOOK_INIT(syslog, smack_syslog), diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index edc52d620f29..b5fb930349a9 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -496,7 +496,7 @@ static int tomoyo_socket_sendmsg(struct socket *sock, struct msghdr *msg, * tomoyo_security_ops is a "struct security_operations" which is used for * registering TOMOYO. */ -static struct security_hook_list tomoyo_hooks[] = { +static struct security_hook_list tomoyo_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(cred_alloc_blank, tomoyo_cred_alloc_blank), LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare), LSM_HOOK_INIT(cred_transfer, tomoyo_cred_transfer), diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 88271a3bf37f..8298e094f4f7 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -428,7 +428,7 @@ int yama_ptrace_traceme(struct task_struct *parent) return rc; } -static struct security_hook_list yama_hooks[] = { +static struct security_hook_list yama_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme), LSM_HOOK_INIT(task_prctl, yama_task_prctl), -- cgit v1.2.3 From 579fc0dc09111c1f8b94742a0ed5a0bb8fdddbb9 Mon Sep 17 00:00:00 2001 From: James Morris Date: Mon, 6 Mar 2017 11:58:08 -0500 Subject: selinux: constify nlmsg permission tables Constify nlmsg permission tables, which are initialized once and then do not change. Signed-off-by: James Morris Signed-off-by: Paul Moore --- security/selinux/nlmsgtab.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c index 2ca9cde939d4..57e2596bdd8a 100644 --- a/security/selinux/nlmsgtab.c +++ b/security/selinux/nlmsgtab.c @@ -28,7 +28,7 @@ struct nlmsg_perm { u32 perm; }; -static struct nlmsg_perm nlmsg_route_perms[] = +static const struct nlmsg_perm nlmsg_route_perms[] = { { RTM_NEWLINK, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, { RTM_DELLINK, NETLINK_ROUTE_SOCKET__NLMSG_WRITE }, @@ -80,7 +80,7 @@ static struct nlmsg_perm nlmsg_route_perms[] = { RTM_GETSTATS, NETLINK_ROUTE_SOCKET__NLMSG_READ }, }; -static struct nlmsg_perm nlmsg_tcpdiag_perms[] = +static const struct nlmsg_perm nlmsg_tcpdiag_perms[] = { { TCPDIAG_GETSOCK, NETLINK_TCPDIAG_SOCKET__NLMSG_READ }, { DCCPDIAG_GETSOCK, NETLINK_TCPDIAG_SOCKET__NLMSG_READ }, @@ -88,7 +88,7 @@ static struct nlmsg_perm nlmsg_tcpdiag_perms[] = { SOCK_DESTROY, NETLINK_TCPDIAG_SOCKET__NLMSG_WRITE }, }; -static struct nlmsg_perm nlmsg_xfrm_perms[] = +static const struct nlmsg_perm nlmsg_xfrm_perms[] = { { XFRM_MSG_NEWSA, NETLINK_XFRM_SOCKET__NLMSG_WRITE }, { XFRM_MSG_DELSA, NETLINK_XFRM_SOCKET__NLMSG_WRITE }, @@ -115,7 +115,7 @@ static struct nlmsg_perm nlmsg_xfrm_perms[] = { XFRM_MSG_MAPPING, NETLINK_XFRM_SOCKET__NLMSG_READ }, }; -static struct nlmsg_perm nlmsg_audit_perms[] = +static const struct nlmsg_perm nlmsg_audit_perms[] = { { AUDIT_GET, NETLINK_AUDIT_SOCKET__NLMSG_READ }, { AUDIT_SET, NETLINK_AUDIT_SOCKET__NLMSG_WRITE }, @@ -136,7 +136,7 @@ static struct nlmsg_perm nlmsg_audit_perms[] = }; -static int nlmsg_perm(u16 nlmsg_type, u32 *perm, struct nlmsg_perm *tab, size_t tabsize) +static int nlmsg_perm(u16 nlmsg_type, u32 *perm, const struct nlmsg_perm *tab, size_t tabsize) { int i, err = -EINVAL; -- cgit v1.2.3 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') 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.3 From f6076f704aa29679fdba114b0f60e71e0884840a Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 10:48:28 +0100 Subject: selinux: Use kmalloc_array() in cond_init_bool_indexes() * A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. * Replace the specification of a data type by a pointer dereference to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 34afeadd9e73..fcfab2635c11 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -176,8 +176,9 @@ void cond_policydb_destroy(struct policydb *p) int cond_init_bool_indexes(struct policydb *p) { kfree(p->bool_val_to_struct); - p->bool_val_to_struct = - kmalloc(p->p_bools.nprim * sizeof(struct cond_bool_datum *), GFP_KERNEL); + p->bool_val_to_struct = kmalloc_array(p->p_bools.nprim, + sizeof(*p->bool_val_to_struct), + GFP_KERNEL); if (!p->bool_val_to_struct) return -ENOMEM; return 0; -- cgit v1.2.3 From e34cfef901badc0e17c2b296a535e7412ef038f3 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 11:00:23 +0100 Subject: selinux: Delete an unnecessary return statement in cond_compute_av() The script "checkpatch.pl" pointed information out like the following. WARNING: void function return statements are not generally useful Thus remove such a statement in the affected function. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index fcfab2635c11..4a3bf29f7565 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -664,5 +664,4 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key, (node->key.specified & AVTAB_XPERMS)) services_compute_xperms_drivers(xperms, node); } - return; } -- cgit v1.2.3 From fb13a312daa11005b0230695a1d6a0b4e2b27069 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 11:22:12 +0100 Subject: selinux: Improve size determinations in four functions Replace the specification of data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 4a3bf29f7565..771c96afe1d5 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -227,7 +227,7 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp) u32 len; int rc; - booldatum = kzalloc(sizeof(struct cond_bool_datum), GFP_KERNEL); + booldatum = kzalloc(sizeof(*booldatum), GFP_KERNEL); if (!booldatum) return -ENOMEM; @@ -332,7 +332,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum goto err; } - list = kzalloc(sizeof(struct cond_av_list), GFP_KERNEL); + list = kzalloc(sizeof(*list), GFP_KERNEL); if (!list) { rc = -ENOMEM; goto err; @@ -421,7 +421,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) goto err; rc = -ENOMEM; - expr = kzalloc(sizeof(struct cond_expr), GFP_KERNEL); + expr = kzalloc(sizeof(*expr), GFP_KERNEL); if (!expr) goto err; @@ -472,7 +472,7 @@ int cond_read_list(struct policydb *p, void *fp) for (i = 0; i < len; i++) { rc = -ENOMEM; - node = kzalloc(sizeof(struct cond_node), GFP_KERNEL); + node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) goto err; -- cgit v1.2.3 From 2f00e680fe25d8d2758879b772d54cb46a78b59d Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 12:06:13 +0100 Subject: selinux: Use kmalloc_array() in hashtab_create() A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/hashtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c index 2cc496149842..dc99fff64ecb 100644 --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -24,7 +24,7 @@ struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void * p->nel = 0; p->hash_value = hash_value; p->keycmp = keycmp; - p->htable = kmalloc(sizeof(*(p->htable)) * size, GFP_KERNEL); + p->htable = kmalloc_array(size, sizeof(*p->htable), GFP_KERNEL); if (p->htable == NULL) { kfree(p); return NULL; -- cgit v1.2.3 From cb8d21e3640f18444c597bddaec156637eacecf8 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 12:36:59 +0100 Subject: selinux: Adjust four checks for null pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script "checkpatch.pl" pointed information out like the following. Comparison to NULL could be written !… Thus fix affected source code places. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/hashtab.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c index dc99fff64ecb..3858706a29fb 100644 --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -17,7 +17,7 @@ struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void * u32 i; p = kzalloc(sizeof(*p), GFP_KERNEL); - if (p == NULL) + if (!p) return p; p->size = size; @@ -25,7 +25,7 @@ struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void * p->hash_value = hash_value; p->keycmp = keycmp; p->htable = kmalloc_array(size, sizeof(*p->htable), GFP_KERNEL); - if (p->htable == NULL) { + if (!p->htable) { kfree(p); return NULL; } @@ -58,7 +58,7 @@ int hashtab_insert(struct hashtab *h, void *key, void *datum) return -EEXIST; newnode = kzalloc(sizeof(*newnode), GFP_KERNEL); - if (newnode == NULL) + if (!newnode) return -ENOMEM; newnode->key = key; newnode->datum = datum; @@ -87,7 +87,7 @@ void *hashtab_search(struct hashtab *h, const void *key) while (cur && h->keycmp(h, key, cur->key) > 0) cur = cur->next; - if (cur == NULL || (h->keycmp(h, key, cur->key) != 0)) + if (!cur || (h->keycmp(h, key, cur->key) != 0)) return NULL; return cur->datum; -- cgit v1.2.3 From ad10a10567e243425d7be35a3d950709371fa048 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 13:08:59 +0100 Subject: selinux: Use kcalloc() in policydb_index() Multiplications for the size determination of memory allocations indicated that array data structures should be processed. Thus use the corresponding function "kcalloc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 9c92f29a38ea..66b9a357fa1b 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -540,23 +540,23 @@ static int policydb_index(struct policydb *p) #endif rc = -ENOMEM; - p->class_val_to_struct = - kzalloc(p->p_classes.nprim * sizeof(*(p->class_val_to_struct)), - GFP_KERNEL); + p->class_val_to_struct = kcalloc(p->p_classes.nprim, + sizeof(*p->class_val_to_struct), + GFP_KERNEL); if (!p->class_val_to_struct) goto out; rc = -ENOMEM; - p->role_val_to_struct = - kzalloc(p->p_roles.nprim * sizeof(*(p->role_val_to_struct)), - GFP_KERNEL); + p->role_val_to_struct = kcalloc(p->p_roles.nprim, + sizeof(*p->role_val_to_struct), + GFP_KERNEL); if (!p->role_val_to_struct) goto out; rc = -ENOMEM; - p->user_val_to_struct = - kzalloc(p->p_users.nprim * sizeof(*(p->user_val_to_struct)), - GFP_KERNEL); + p->user_val_to_struct = kcalloc(p->p_users.nprim, + sizeof(*p->user_val_to_struct), + GFP_KERNEL); if (!p->user_val_to_struct) goto out; -- cgit v1.2.3 From b4e4686f65a3092f63ed01c887d9f56714d29f4a Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 14:00:02 +0100 Subject: selinux: Delete an unnecessary return statement in policydb_destroy() The script "checkpatch.pl" pointed information out like the following. WARNING: void function return statements are not generally useful Thus remove such a statement in the affected function. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 66b9a357fa1b..bccc9acf6bc5 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -880,8 +880,6 @@ void policydb_destroy(struct policydb *p) ebitmap_destroy(&p->filename_trans_ttypes); ebitmap_destroy(&p->policycaps); ebitmap_destroy(&p->permissive_map); - - return; } /* -- cgit v1.2.3 From 3a0aa56518432a1a598ac3e48a93f2e99c66a393 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 16:34:25 +0100 Subject: selinux: Return directly after a failed next_entry() in genfs_read() Return directly after a call of the function "next_entry" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index bccc9acf6bc5..375e304070e1 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2006,7 +2006,7 @@ static int genfs_read(struct policydb *p, void *fp) rc = next_entry(buf, fp, sizeof(u32)); if (rc) - goto out; + return rc; nel = le32_to_cpu(buf[0]); for (i = 0; i < nel; i++) { -- cgit v1.2.3 From 315e01ada8047cba0520ecca050ad8f5237abb41 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 17:43:47 +0100 Subject: selinux: One function call less in genfs_read() after null pointer detection Call the function "kfree" at the end only after it was determined that the local variable "newgenfs" contained a non-null pointer. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 375e304070e1..4390558464c5 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2098,9 +2098,10 @@ static int genfs_read(struct policydb *p, void *fp) } rc = 0; out: - if (newgenfs) + if (newgenfs) { kfree(newgenfs->fstype); - kfree(newgenfs); + kfree(newgenfs); + } ocontext_destroy(newc, OCON_FSUSE); return rc; -- cgit v1.2.3 From 02fcef27cc908e22ddb68d30ad1b7fd9ac3a1c24 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 19:02:42 +0100 Subject: selinux: Delete an unnecessary variable assignment in filename_trans_read() The local variable "ft" was set to a null pointer despite of an immediate reassignment. Thus remove this statement from the beginning of a loop. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 4390558464c5..7131251be628 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1929,7 +1929,6 @@ static int filename_trans_read(struct policydb *p, void *fp) nel = le32_to_cpu(buf[0]); for (i = 0; i < nel; i++) { - ft = NULL; otype = NULL; name = NULL; -- cgit v1.2.3 From 57152a5be08e240654993984a0f6e54254882626 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 19:35:59 +0100 Subject: selinux: Return directly after a failed next_entry() in range_read() Return directly after a call of the function "next_entry" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 7131251be628..6c093018ae8d 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1852,7 +1852,7 @@ static int range_read(struct policydb *p, void *fp) rc = next_entry(buf, fp, sizeof(u32)); if (rc) - goto out; + return rc; nel = le32_to_cpu(buf[0]); for (i = 0; i < nel; i++) { -- cgit v1.2.3 From 9c312e79d6afb673114ebe7aabd5594e79827eea Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 20:40:12 +0100 Subject: selinux: Delete an unnecessary variable initialisation in range_read() The local variable "rt" will be set to an appropriate pointer a bit later. Thus omit the explicit initialisation at the beginning which became unnecessary with a previous update step. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 6c093018ae8d..a8389396e9a9 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1841,7 +1841,7 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name) static int range_read(struct policydb *p, void *fp) { - struct range_trans *rt = NULL; + struct range_trans *rt; struct mls_range *r = NULL; int i, rc; __le32 buf[2]; -- cgit v1.2.3 From 7f6d0ad8b771a4936f448180de3bbfad92be34dc Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 21:20:43 +0100 Subject: selinux: Return directly after a failed kzalloc() in cat_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index a8389396e9a9..36285d12c2e9 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1637,10 +1637,9 @@ static int cat_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[3]; u32 len; - rc = -ENOMEM; catdatum = kzalloc(sizeof(*catdatum), GFP_ATOMIC); if (!catdatum) - goto bad; + return -ENOMEM; rc = next_entry(buf, fp, sizeof buf); if (rc) -- cgit v1.2.3 From 3c354d7d7bd5e788c3d94969e4a85648d29d9d12 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 21:42:02 +0100 Subject: selinux: Return directly after a failed kzalloc() in sens_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 36285d12c2e9..aa1ce7ce3524 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1595,10 +1595,9 @@ static int sens_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[2]; u32 len; - rc = -ENOMEM; levdatum = kzalloc(sizeof(*levdatum), GFP_ATOMIC); if (!levdatum) - goto bad; + return -ENOMEM; rc = next_entry(buf, fp, sizeof buf); if (rc) -- cgit v1.2.3 From b5921191005d050d55cf4f3b10f60110f7ed2c24 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 21:52:55 +0100 Subject: selinux: Improve another size determination in sens_read() Replace the specification of a data type by a pointer dereference as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index aa1ce7ce3524..4759c22d1ae6 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1611,7 +1611,7 @@ static int sens_read(struct policydb *p, struct hashtab *h, void *fp) goto bad; rc = -ENOMEM; - levdatum->level = kmalloc(sizeof(struct mls_level), GFP_ATOMIC); + levdatum->level = kmalloc(sizeof(*levdatum->level), GFP_ATOMIC); if (!levdatum->level) goto bad; -- cgit v1.2.3 From 4bd9f07b89f1a300f96b4863c3a8a4ec36991930 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 22:08:22 +0100 Subject: selinux: Return directly after a failed kzalloc() in user_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 4759c22d1ae6..9a90953974b8 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1544,10 +1544,9 @@ static int user_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[3]; u32 len; - rc = -ENOMEM; usrdatum = kzalloc(sizeof(*usrdatum), GFP_KERNEL); if (!usrdatum) - goto bad; + return -ENOMEM; if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) to_read = 3; -- cgit v1.2.3 From 549fe69ee5c7a7f55c34555032961a2265e6e713 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 22:15:54 +0100 Subject: selinux: Return directly after a failed kzalloc() in type_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 9a90953974b8..fd58de5a83ad 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1469,10 +1469,9 @@ static int type_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[4]; u32 len; - rc = -ENOMEM; typdatum = kzalloc(sizeof(*typdatum), GFP_KERNEL); if (!typdatum) - goto bad; + return -ENOMEM; if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) to_read = 4; -- cgit v1.2.3 From ea6e2f7d12921f336def7398805ee3b1619e2f4b Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 22:20:25 +0100 Subject: selinux: Return directly after a failed kzalloc() in role_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index fd58de5a83ad..30f29c669e32 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1412,10 +1412,9 @@ static int role_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[3]; u32 len; - rc = -ENOMEM; role = kzalloc(sizeof(*role), GFP_KERNEL); if (!role) - goto bad; + return -ENOMEM; if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) to_read = 3; -- cgit v1.2.3 From df4a14dfb484f95d81126e481e66b6e22eec49e8 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 14 Jan 2017 22:30:51 +0100 Subject: selinux: Return directly after a failed kzalloc() in class_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 30f29c669e32..edf173ed05f8 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1318,10 +1318,9 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp) u32 len, len2, ncons, nel; int i, rc; - rc = -ENOMEM; cladatum = kzalloc(sizeof(*cladatum), GFP_KERNEL); if (!cladatum) - goto bad; + return -ENOMEM; rc = next_entry(buf, fp, sizeof(u32)*6); if (rc) -- cgit v1.2.3 From 442ca4d656645505346017c37ac137cde680bf38 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 11:15:19 +0100 Subject: selinux: Return directly after a failed kzalloc() in common_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index edf173ed05f8..99ee0ee2d92a 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1152,10 +1152,9 @@ static int common_read(struct policydb *p, struct hashtab *h, void *fp) u32 len, nel; int i, rc; - rc = -ENOMEM; comdatum = kzalloc(sizeof(*comdatum), GFP_KERNEL); if (!comdatum) - goto bad; + return -ENOMEM; rc = next_entry(buf, fp, sizeof buf); if (rc) -- cgit v1.2.3 From 7befb7514e5d53026e9fe4a6548f118a65a20a4f Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 11:20:13 +0100 Subject: selinux: Return directly after a failed kzalloc() in perm_read() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 99ee0ee2d92a..5ca2d26ecf7f 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1118,10 +1118,9 @@ static int perm_read(struct policydb *p, struct hashtab *h, void *fp) __le32 buf[2]; u32 len; - rc = -ENOMEM; perdatum = kzalloc(sizeof(*perdatum), GFP_KERNEL); if (!perdatum) - goto bad; + return -ENOMEM; rc = next_entry(buf, fp, sizeof buf); if (rc) -- cgit v1.2.3 From ebd2b47ba52760e9653456ba19032f79d734a343 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 12:10:09 +0100 Subject: selinux: Return directly after a failed kzalloc() in roles_init() Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 5ca2d26ecf7f..658247f98dc1 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -178,10 +178,9 @@ static int roles_init(struct policydb *p) int rc; struct role_datum *role; - rc = -ENOMEM; role = kzalloc(sizeof(*role), GFP_KERNEL); if (!role) - goto out; + return -ENOMEM; rc = -EINVAL; role->value = ++p->p_roles.nprim; -- cgit v1.2.3 From b380f783772eeb66a0da85549a85aadbe5f8e76d Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 13:13:19 +0100 Subject: selinux: Use kmalloc_array() in sidtab_init() A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/sidtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 5840a35155fc..c9533b21942b 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -18,7 +18,7 @@ int sidtab_init(struct sidtab *s) { int i; - s->htable = kmalloc(sizeof(*(s->htable)) * SIDTAB_SIZE, GFP_ATOMIC); + s->htable = kmalloc_array(SIDTAB_SIZE, sizeof(*s->htable), GFP_ATOMIC); if (!s->htable) return -ENOMEM; for (i = 0; i < SIDTAB_SIZE; i++) -- cgit v1.2.3 From 8ee4586ca5fe6c2a00f6a39f828f54c8f6cda472 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 13:30:20 +0100 Subject: selinux: Adjust two checks for null pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script "checkpatch.pl" pointed information out like the following. Comparison to NULL could be written !… Thus fix affected source code places. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/ss/sidtab.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index c9533b21942b..f6915f257486 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -54,7 +54,7 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) } newnode = kmalloc(sizeof(*newnode), GFP_ATOMIC); - if (newnode == NULL) { + if (!newnode) { rc = -ENOMEM; goto out; } @@ -98,7 +98,7 @@ static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force) if (force && cur && sid == cur->sid && cur->context.len) return &cur->context; - if (cur == NULL || sid != cur->sid || cur->context.len) { + if (!cur || sid != cur->sid || cur->context.len) { /* Remap invalid SIDs to the unlabeled SID. */ sid = SECINITSID_UNLABELED; hvalue = SIDTAB_HASH(sid); -- cgit v1.2.3 From 710a0647ba955abd25460c36a09d80fdbe878273 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sun, 15 Jan 2017 14:04:53 +0100 Subject: selinuxfs: Use seq_puts() in sel_avc_stats_seq_show() A string which did not contain data format specifications should be put into a sequence. Thus use the corresponding function "seq_puts". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index cb3fd98fb05a..ce7171884223 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1456,10 +1456,10 @@ static int sel_avc_stats_seq_show(struct seq_file *seq, void *v) { struct avc_cache_stats *st = v; - if (v == SEQ_START_TOKEN) - seq_printf(seq, "lookups hits misses allocations reclaims " - "frees\n"); - else { + if (v == SEQ_START_TOKEN) { + seq_puts(seq, + "lookups hits misses allocations reclaims frees\n"); + } else { unsigned int lookups = st->lookups; unsigned int misses = st->misses; unsigned int hits = lookups - misses; -- cgit v1.2.3 From 342e91578eb6909529bc7095964cd44b9c057c4e Mon Sep 17 00:00:00 2001 From: Matthias Kaehlcke Date: Thu, 16 Mar 2017 15:26:52 -0700 Subject: selinux: Remove unnecessary check of array base in selinux_set_mapping() 'perms' will never be NULL since it isn't a plain pointer but an array of u32 values. This fixes the following warning when building with clang: security/selinux/ss/services.c:158:16: error: address of array 'p_in->perms' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] while (p_in->perms && p_in->perms[k]) { Signed-off-by: Matthias Kaehlcke Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index b4aa491a0a23..60d9b0252321 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -157,7 +157,7 @@ static int selinux_set_mapping(struct policydb *pol, } k = 0; - while (p_in->perms && p_in->perms[k]) { + while (p_in->perms[k]) { /* An empty permission string skips ahead */ if (!*p_in->perms[k]) { k++; -- cgit v1.2.3 From cae303df3f379f04ce7efadb2e30de460918b302 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 31 Mar 2017 18:21:18 +0300 Subject: selinux: Fix an uninitialized variable bug We removed this initialization as a cleanup but it is probably required. The concern is that "nel" can be zero. I'm not an expert on SELinux code but I think it looks possible to write an SELinux policy which triggers this bug. GCC doesn't catch this, but my static checker does. Fixes: 9c312e79d6af ("selinux: Delete an unnecessary variable initialisation in range_read()") Signed-off-by: Dan Carpenter Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 658247f98dc1..0080122760ad 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1832,7 +1832,7 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name) static int range_read(struct policydb *p, void *fp) { - struct range_trans *rt; + struct range_trans *rt = NULL; struct mls_range *r = NULL; int i, rc; __le32 buf[2]; -- cgit v1.2.3