smack: fix bug: unprivileged task can create labels

If an unprivileged task is allowed to relabel itself
(/smack/relabel-self is not empty),
it can freely create new labels by writing their
names into own /proc/PID/attr/smack/current

This occurs because do_setattr() imports
the provided label in advance,
before checking "relabel-self" list.

This change ensures that the "relabel-self" list
is checked before importing the label.

Fixes: 38416e53936e ("Smack: limited capability for changing process label")
Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 30f20fef3331e060e21c85f7731389fec069ba96..159ccc1d9847cad139eb950fc193968b7de6358e 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3778,8 +3778,8 @@ static int do_setattr(u64 attr, void *value, size_t size)
        struct task_smack *tsp = smack_cred(current_cred());
        struct cred *new;
        struct smack_known *skp;
-       struct smack_known_list_elem *sklep;
-       int rc;
+       char *labelstr;
+       int rc = 0;
 
        if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
                return -EPERM;
@@ -3790,28 +3790,41 @@ static int do_setattr(u64 attr, void *value, size_t size)
        if (attr != LSM_ATTR_CURRENT)
                return -EOPNOTSUPP;
 
-       skp = smk_import_entry(value, size);
-       if (IS_ERR(skp))
-               return PTR_ERR(skp);
+       labelstr = smk_parse_smack(value, size);
+       if (IS_ERR(labelstr))
+               return PTR_ERR(labelstr);
 
        /*
         * No process is ever allowed the web ("@") label
         * and the star ("*") label.
         */
-       if (skp == &smack_known_web || skp == &smack_known_star)
-               return -EINVAL;
+       if (labelstr[1] == '\0' /* '@', '*' */) {
+               const char c = labelstr[0];
+
+               if (c == *smack_known_web.smk_known ||
+                   c == *smack_known_star.smk_known) {
+                       rc = -EPERM;
+                       goto free_labelstr;
+               }
+       }
 
        if (!smack_privileged(CAP_MAC_ADMIN)) {
-               rc = -EPERM;
+               const struct smack_known_list_elem *sklep;
                list_for_each_entry(sklep, &tsp->smk_relabel, list)
-                       if (sklep->smk_label == skp) {
-                               rc = 0;
-                               break;
-                       }
-               if (rc)
-                       return rc;
+                       if (strcmp(sklep->smk_label->smk_known, labelstr) == 0)
+                               goto free_labelstr;
+               rc = -EPERM;
        }
 
+free_labelstr:
+       kfree(labelstr);
+       if (rc)
+               return -EPERM;
+
+       skp = smk_import_entry(value, size);
+       if (IS_ERR(skp))
+               return PTR_ERR(skp);
+
        new = prepare_creds();
        if (new == NULL)
                return -ENOMEM;