From 6326948f940dc3f77066d5cdc44ba6afe67830c0 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 29 Sep 2021 11:01:21 -0400 Subject: lsm: security_task_getsecid_subj() -> security_current_getsecid_subj() The security_task_getsecid_subj() LSM hook invites misuse by allowing callers to specify a task even though the hook is only safe when the current task is referenced. Fix this by removing the task_struct argument to the hook, requiring LSM implementations to use the current task. While we are changing the hook declaration we also rename the function to security_current_getsecid_subj() in an effort to reinforce that the hook captures the subjective credentials of the current task and not an arbitrary task on the system. Reviewed-by: Serge Hallyn Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- security/selinux/hooks.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 62d30c0a30c2..726175254f60 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -229,19 +229,6 @@ static inline u32 cred_sid(const struct cred *cred) return tsec->sid; } -/* - * get the subjective security ID of a task - */ -static inline u32 task_sid_subj(const struct task_struct *task) -{ - u32 sid; - - rcu_read_lock(); - sid = cred_sid(rcu_dereference(task->cred)); - rcu_read_unlock(); - return sid; -} - /* * get the objective security ID of a task */ @@ -4205,9 +4192,9 @@ static int selinux_task_getsid(struct task_struct *p) PROCESS__GETSESSION, NULL); } -static void selinux_task_getsecid_subj(struct task_struct *p, u32 *secid) +static void selinux_current_getsecid_subj(u32 *secid) { - *secid = task_sid_subj(p); + *secid = current_sid(); } static void selinux_task_getsecid_obj(struct task_struct *p, u32 *secid) @@ -7159,7 +7146,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid), LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid), LSM_HOOK_INIT(task_getsid, selinux_task_getsid), - LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid_subj), + LSM_HOOK_INIT(current_getsecid_subj, selinux_current_getsecid_subj), LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid_obj), LSM_HOOK_INIT(task_setnice, selinux_task_setnice), LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio), -- cgit From 5fe375728983b3fca6b958434a2e3f547bbbb2aa Mon Sep 17 00:00:00 2001 From: Xiu Jianfeng Date: Thu, 2 Dec 2021 15:35:33 +0800 Subject: selinux: Use struct_size() helper in kmalloc() Make use of struct_size() helper instead of an open-coded calculation. Link: https://github.com/KSPP/linux/issues/160 Signed-off-by: Xiu Jianfeng Signed-off-by: Paul Moore --- security/selinux/ss/sidtab.c | 2 +- security/selinux/xfrm.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 656d50b09f76..293ec048af08 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -570,7 +570,7 @@ void sidtab_sid2str_put(struct sidtab *s, struct sidtab_entry *entry, goto out_unlock; } - cache = kmalloc(sizeof(struct sidtab_str_cache) + str_len, GFP_ATOMIC); + cache = kmalloc(struct_size(cache, str, str_len), GFP_ATOMIC); if (!cache) goto out_unlock; diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index be83e5ce4469..90697317895f 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -89,7 +89,7 @@ static int selinux_xfrm_alloc_user(struct xfrm_sec_ctx **ctxp, if (str_len >= PAGE_SIZE) return -ENOMEM; - ctx = kmalloc(sizeof(*ctx) + str_len + 1, gfp); + ctx = kmalloc(struct_size(ctx, ctx_str, str_len + 1), gfp); if (!ctx) return -ENOMEM; @@ -360,7 +360,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, if (rc) return rc; - ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC); + ctx = kmalloc(struct_size(ctx, ctx_str, str_len), GFP_ATOMIC); if (!ctx) { rc = -ENOMEM; goto out; -- cgit From 52f982f00b220d097a71a23c149a1d18efc08e63 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 6 Dec 2021 14:24:06 +0100 Subject: security,selinux: remove security_add_mnt_opt() Its last user has been removed in commit f2aedb713c28 ("NFS: Add fs_context support."). Signed-off-by: Ondrej Mosnacek Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- security/selinux/hooks.c | 39 --------------------------------------- 1 file changed, 39 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 726175254f60..818ce976ff6c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1010,44 +1010,6 @@ Einval: return -EINVAL; } -static int selinux_add_mnt_opt(const char *option, const char *val, int len, - void **mnt_opts) -{ - int token = Opt_error; - int rc, i; - - for (i = 0; i < ARRAY_SIZE(tokens); i++) { - if (strcmp(option, tokens[i].name) == 0) { - token = tokens[i].opt; - break; - } - } - - if (token == Opt_error) - return -EINVAL; - - if (token != Opt_seclabel) { - val = kmemdup_nul(val, len, GFP_KERNEL); - if (!val) { - rc = -ENOMEM; - goto free_opt; - } - } - rc = selinux_add_opt(token, val, mnt_opts); - if (unlikely(rc)) { - kfree(val); - goto free_opt; - } - return rc; - -free_opt: - if (*mnt_opts) { - selinux_free_mnt_opts(*mnt_opts); - *mnt_opts = NULL; - } - return rc; -} - static int show_sid(struct seq_file *m, u32 sid) { char *context = NULL; @@ -7285,7 +7247,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(fs_context_dup, selinux_fs_context_dup), LSM_HOOK_INIT(fs_context_parse_param, selinux_fs_context_parse_param), LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts), - LSM_HOOK_INIT(sb_add_mnt_opt, selinux_add_mnt_opt), #ifdef CONFIG_SECURITY_NETWORK_XFRM LSM_HOOK_INIT(xfrm_policy_clone_security, selinux_xfrm_policy_clone), #endif -- cgit From 2e08df3c7c4e4e74e3dd5104c100f0bf6288aaa8 Mon Sep 17 00:00:00 2001 From: Bernard Zhao Date: Fri, 10 Dec 2021 04:03:58 -0800 Subject: selinux: fix potential memleak in selinux_add_opt() This patch try to fix potential memleak in error branch. Fixes: ba6418623385 ("selinux: new helper - selinux_add_opt()") Signed-off-by: Bernard Zhao [PM: tweak the subject line, add Fixes tag] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 818ce976ff6c..8ef63b7af855 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -970,18 +970,22 @@ out: static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; + bool is_alloc_opts = false; if (token == Opt_seclabel) /* eaten and completely ignored */ return 0; + if (!s) + return -ENOMEM; + if (!opts) { opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); if (!opts) return -ENOMEM; *mnt_opts = opts; + is_alloc_opts = true; } - if (!s) - return -ENOMEM; + switch (token) { case Opt_context: if (opts->context || opts->defcontext) @@ -1006,6 +1010,10 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) } return 0; Einval: + if (is_alloc_opts) { + kfree(opts); + *mnt_opts = NULL; + } pr_warn(SEL_MOUNT_FAIL_MSG); return -EINVAL; } -- cgit From 6cd9d4b97891560b61681cad9cc4307ce0719abc Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 21 Dec 2021 15:01:29 -0500 Subject: selinux: minor tweaks to selinux_add_opt() Two minor edits to selinux_add_opt(): use "sizeof(*ptr)" instead of "sizeof(type)" in the kzalloc() call, and rename the "Einval" jump target to "err" for the sake of consistency. Signed-off-by: Paul Moore --- security/selinux/hooks.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'security/selinux') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8ef63b7af855..904f9c23f0f6 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -972,14 +972,14 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) struct selinux_mnt_opts *opts = *mnt_opts; bool is_alloc_opts = false; - if (token == Opt_seclabel) /* eaten and completely ignored */ + if (token == Opt_seclabel) + /* eaten and completely ignored */ return 0; - if (!s) return -ENOMEM; if (!opts) { - opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); + opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) return -ENOMEM; *mnt_opts = opts; @@ -989,27 +989,29 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) switch (token) { case Opt_context: if (opts->context || opts->defcontext) - goto Einval; + goto err; opts->context = s; break; case Opt_fscontext: if (opts->fscontext) - goto Einval; + goto err; opts->fscontext = s; break; case Opt_rootcontext: if (opts->rootcontext) - goto Einval; + goto err; opts->rootcontext = s; break; case Opt_defcontext: if (opts->context || opts->defcontext) - goto Einval; + goto err; opts->defcontext = s; break; } + return 0; -Einval: + +err: if (is_alloc_opts) { kfree(opts); *mnt_opts = NULL; -- cgit