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 <serge@hallyn.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index df8de62f4710f5b1e49c21d07c50ce506202ce1d..ae2228f0711dddf0f4cb5178f065090f99843b9c 100644 (file)
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -206,8 +206,7 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old,
 LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
 LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
 LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
-LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
-        struct task_struct *p, u32 *secid)
+LSM_HOOK(void, LSM_RET_VOID, current_getsecid_subj, u32 *secid)
 LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
         struct task_struct *p, u32 *secid)
 LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d45b6f6e27fdafaf3ed61767a15cc5b8846ca718..52c1990644b98a8592ff76b8eb3ae3adc0abc28e 100644 (file)
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -719,11 +719,9 @@
  *     @p.
  *     @p contains the task_struct for the process.
  *     Return 0 if permission is granted.
- * @task_getsecid_subj:
- *     Retrieve the subjective security identifier of the task_struct in @p
- *     and return it in @secid.  Special care must be taken to ensure that @p
- *     is the either the "current" task, or the caller has exclusive access
- *     to @p.
+ * @current_getsecid_subj:
+ *     Retrieve the subjective security identifier of the current task and
+ *     return it in @secid.
  *     In case of failure, @secid will be set to zero.
  * @task_getsecid_obj:
  *     Retrieve the objective security identifier of the task_struct in @p

diff --git a/include/linux/security.h b/include/linux/security.h
index bbf44a46683262c803ce92a3f76676362b4cc9c8..bb301963e33398eed5c9c0b55b5c5789a2e0274f 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -418,7 +418,7 @@ int security_task_fix_setgid(struct cred *new, const struct cred *old,
 int security_task_setpgid(struct task_struct *p, pid_t pgid);
 int security_task_getpgid(struct task_struct *p);
 int security_task_getsid(struct task_struct *p);
-void security_task_getsecid_subj(struct task_struct *p, u32 *secid);
+void security_current_getsecid_subj(u32 *secid);
 void security_task_getsecid_obj(struct task_struct *p, u32 *secid);
 int security_task_setnice(struct task_struct *p, int nice);
 int security_task_setioprio(struct task_struct *p, int ioprio);
@@ -1119,7 +1119,7 @@ static inline int security_task_getsid(struct task_struct *p)
        return 0;
 }
 
-static inline void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
+static inline void security_current_getsecid_subj(u32 *secid)
 {
        *secid = 0;
 }

diff --git a/kernel/audit.c b/kernel/audit.c
index 121d37e700a62b53854c06199d9a89850ec39dd4..d4084751cfe69081ddb87a045d2477b8299063bb 100644 (file)
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2132,7 +2132,7 @@ int audit_log_task_context(struct audit_buffer *ab)
        int error;
        u32 sid;
 
-       security_task_getsecid_subj(current, &sid);
+       security_current_getsecid_subj(&sid);
        if (!sid)
                return 0;
 
@@ -2353,7 +2353,7 @@ int audit_signal_info(int sig, struct task_struct *t)
                        audit_sig_uid = auid;
                else
                        audit_sig_uid = uid;
-               security_task_getsecid_subj(current, &audit_sig_sid);
+               security_current_getsecid_subj(&audit_sig_sid);
        }
 
        return audit_signal_info_syscall(t);

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d75acb014ccdcba2a0bed0646323f5427757e493..4173e771650c8beea749484623b8f5a5824cbfc5 100644 (file)
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1368,8 +1368,7 @@ int audit_filter(int msgtype, unsigned int listtype)
                        case AUDIT_SUBJ_SEN:
                        case AUDIT_SUBJ_CLR:
                                if (f->lsm_rule) {
-                                       security_task_getsecid_subj(current,
-                                                                   &sid);
+                                       security_current_getsecid_subj(&sid);
                                        result = security_audit_rule_match(sid,
                                                   f->type, f->op, f->lsm_rule);
                                }

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index b517947bfa48d97ab7e4b6812114b8fc0b533d42..fce5d43a933f0000df654edd3a6d95481dd319e8 100644 (file)
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -666,7 +666,16 @@ static int audit_filter_rules(struct task_struct *tsk,
                           logged upon error */
                        if (f->lsm_rule) {
                                if (need_sid) {
-                                       security_task_getsecid_subj(tsk, &sid);
+                                       /* @tsk should always be equal to
+                                        * @current with the exception of
+                                        * fork()/copy_process() in which case
+                                        * the new @tsk creds are still a dup
+                                        * of @current's creds so we can still
+                                        * use security_current_getsecid_subj()
+                                        * here even though it always refs
+                                        * @current's creds
+                                        */
+                                       security_current_getsecid_subj(&sid);
                                        need_sid = 0;
                                }
                                result = security_audit_rule_match(sid, f->type,

diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 566ba4397ee4007e8ed170bb775253f4b15f9079..8490e46359ae0a4eac91a1b54f46375c43fb0c08 100644 (file)
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1537,7 +1537,7 @@ int __init netlbl_unlabel_defconf(void)
        /* Only the kernel is allowed to call this function and the only time
         * it is called is at bootup before the audit subsystem is reporting
         * messages so don't worry to much about these values. */
-       security_task_getsecid_subj(current, &audit_info.secid);
+       security_current_getsecid_subj(&audit_info.secid);
        audit_info.loginuid = GLOBAL_ROOT_UID;
        audit_info.sessionid = 0;
 

diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
index 6190cbf94bf0d3125d020dc6c7d6b69c1ef134e1..d6c5b31eb4eb8dfa693bf63590081614540cb9ff 100644 (file)
--- a/net/netlabel/netlabel_user.h
+++ b/net/netlabel/netlabel_user.h
@@ -32,7 +32,7 @@
  */
 static inline void netlbl_netlink_auditinfo(struct netlbl_audit *audit_info)
 {
-       security_task_getsecid_subj(current, &audit_info->secid);
+       security_current_getsecid_subj(&audit_info->secid);
        audit_info->loginuid = audit_get_loginuid(current);
        audit_info->sessionid = audit_get_sessionid(current);
 }

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 0d6585056f3df5f1e872b91217ac570b2f0f9ac7..4f0eecb67dde09094d79d4e1483bea8baae5417d 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -728,7 +728,14 @@ static void apparmor_bprm_committed_creds(struct linux_binprm *bprm)
        return;
 }
 
-static void apparmor_task_getsecid(struct task_struct *p, u32 *secid)
+static void apparmor_current_getsecid_subj(u32 *secid)
+{
+       struct aa_label *label = aa_get_current_label();
+       *secid = label->secid;
+       aa_put_label(label);
+}
+
+static void apparmor_task_getsecid_obj(struct task_struct *p, u32 *secid)
 {
        struct aa_label *label = aa_get_task_label(p);
        *secid = label->secid;
@@ -1252,8 +1259,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
 
        LSM_HOOK_INIT(task_free, apparmor_task_free),
        LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
-       LSM_HOOK_INIT(task_getsecid_subj, apparmor_task_getsecid),
-       LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid),
+       LSM_HOOK_INIT(current_getsecid_subj, apparmor_current_getsecid_subj),
+       LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid_obj),
        LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
        LSM_HOOK_INIT(task_kill, apparmor_task_kill),
 

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index dbba51583e7c13143707ebaf194e3e73ef93c04a..17232bbfb9f96c541804e0c0e0a2ff45f73943f1 100644 (file)
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -76,7 +76,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
        if (!ima_appraise)
                return 0;
 
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        return ima_match_policy(mnt_userns, inode, current_cred(), secid,
                                func, mask, IMA_APPRAISE | IMA_HASH, NULL,
                                NULL, NULL, NULL);

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 465865412100bf656038ce151c6f60006cf3a382..8c6e4514d4944c0ba7c3081733846ffde70d2f63 100644 (file)
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -408,7 +408,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
        u32 secid;
 
        if (file && (prot & PROT_EXEC)) {
-               security_task_getsecid_subj(current, &secid);
+               security_current_getsecid_subj(&secid);
                return process_measurement(file, current_cred(), secid, NULL,
                                           0, MAY_EXEC, MMAP_CHECK);
        }
@@ -446,7 +446,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
            !(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
                return 0;
 
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        inode = file_inode(vma->vm_file);
        action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
                                current_cred(), secid, MAY_EXEC, MMAP_CHECK,
@@ -487,7 +487,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
        int ret;
        u32 secid;
 
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
                                  MAY_EXEC, BPRM_CHECK);
        if (ret)
@@ -512,7 +512,7 @@ int ima_file_check(struct file *file, int mask)
 {
        u32 secid;
 
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        return process_measurement(file, current_cred(), secid, NULL, 0,
                                   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
                                           MAY_APPEND), FILE_CHECK);
@@ -709,7 +709,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
 
        /* Read entire file for all partial reads. */
        func = read_idmap[read_id] ?: FILE_CHECK;
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        return process_measurement(file, current_cred(), secid, NULL,
                                   0, MAY_READ, func);
 }
@@ -752,7 +752,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
        }
 
        func = read_idmap[read_id] ?: FILE_CHECK;
-       security_task_getsecid_subj(current, &secid);
+       security_current_getsecid_subj(&secid);
        return process_measurement(file, current_cred(), secid, buf, size,
                                   MAY_READ, func);
 }
@@ -905,7 +905,7 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
         * buffer measurements.
         */
        if (func) {
-               security_task_getsecid_subj(current, &secid);
+               security_current_getsecid_subj(&secid);
                action = ima_get_action(mnt_userns, inode, current_cred(),
                                        secid, 0, func, &pcr, &template,
                                        func_data, NULL);

diff --git a/security/security.c b/security/security.c
index c88167a414b4116d10ba1c87898f91a62726a9fc..edb922b8bf4a0cc354ac082e2035d818382ef40d 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -1808,12 +1808,12 @@ int security_task_getsid(struct task_struct *p)
        return call_int_hook(task_getsid, 0, p);
 }
 
-void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
+void security_current_getsecid_subj(u32 *secid)
 {
        *secid = 0;
-       call_void_hook(task_getsecid_subj, p, secid);
+       call_void_hook(current_getsecid_subj, secid);
 }
-EXPORT_SYMBOL(security_task_getsecid_subj);
+EXPORT_SYMBOL(security_current_getsecid_subj);
 
 void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
 {

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 62d30c0a30c291753bdf1df23d7909394b204286..726175254f60d55e84bfba02da3fbe9da562719b 100644 (file)
--- 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),

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 99c3422596ab9e6a423375ca5b47f2db8ea4ac17..fc837dcebf96e05ac38dfbb09678409901b5f398 100644 (file)
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -389,22 +389,6 @@ static inline struct smack_known *smk_of_task(const struct task_smack *tsp)
        return tsp->smk_task;
 }
 
-static inline struct smack_known *smk_of_task_struct_subj(
-                                               const struct task_struct *t)
-{
-       struct smack_known *skp;
-       const struct cred *cred;
-
-       rcu_read_lock();
-
-       cred = rcu_dereference(t->cred);
-       skp = smk_of_task(smack_cred(cred));
-
-       rcu_read_unlock();
-
-       return skp;
-}
-
 static inline struct smack_known *smk_of_task_struct_obj(
                                                const struct task_struct *t)
 {

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index efd35b07c7f88f713131ae7cdd9f3f2b4a0ce4fc..14b279cc75c964b7fb5f5001d41bd20fb23940a7 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2067,15 +2067,14 @@ static int smack_task_getsid(struct task_struct *p)
 }
 
 /**
- * smack_task_getsecid_subj - get the subjective secid of the task
- * @p: the task
+ * smack_current_getsecid_subj - get the subjective secid of the current task
  * @secid: where to put the result
  *
  * Sets the secid to contain a u32 version of the task's subjective smack label.
  */
-static void smack_task_getsecid_subj(struct task_struct *p, u32 *secid)
+static void smack_current_getsecid_subj(u32 *secid)
 {
-       struct smack_known *skp = smk_of_task_struct_subj(p);
+       struct smack_known *skp = smk_of_current();
 
        *secid = skp->smk_secid;
 }
@@ -4807,7 +4806,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
        LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
        LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
        LSM_HOOK_INIT(task_getsid, smack_task_getsid),
-       LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid_subj),
+       LSM_HOOK_INIT(current_getsecid_subj, smack_current_getsecid_subj),
        LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid_obj),
        LSM_HOOK_INIT(task_setnice, smack_task_setnice),
        LSM_HOOK_INIT(task_setioprio, smack_task_setioprio),