Skip to content

Commit c147e13

Browse files
goongascschaufler
authored andcommitted
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: 38416e5 ("Smack: limited capability for changing process label") Signed-off-by: Konstantin Andreev <andreev@swemel.ru> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
1 parent 78fc6a9 commit c147e13

1 file changed

Lines changed: 27 additions & 14 deletions

File tree

security/smack/smack_lsm.c

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3778,8 +3778,8 @@ static int do_setattr(u64 attr, void *value, size_t size)
37783778
struct task_smack *tsp = smack_cred(current_cred());
37793779
struct cred *new;
37803780
struct smack_known *skp;
3781-
struct smack_known_list_elem *sklep;
3782-
int rc;
3781+
char *labelstr;
3782+
int rc = 0;
37833783

37843784
if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
37853785
return -EPERM;
@@ -3790,28 +3790,41 @@ static int do_setattr(u64 attr, void *value, size_t size)
37903790
if (attr != LSM_ATTR_CURRENT)
37913791
return -EOPNOTSUPP;
37923792

3793-
skp = smk_import_entry(value, size);
3794-
if (IS_ERR(skp))
3795-
return PTR_ERR(skp);
3793+
labelstr = smk_parse_smack(value, size);
3794+
if (IS_ERR(labelstr))
3795+
return PTR_ERR(labelstr);
37963796

37973797
/*
37983798
* No process is ever allowed the web ("@") label
37993799
* and the star ("*") label.
38003800
*/
3801-
if (skp == &smack_known_web || skp == &smack_known_star)
3802-
return -EINVAL;
3801+
if (labelstr[1] == '\0' /* '@', '*' */) {
3802+
const char c = labelstr[0];
3803+
3804+
if (c == *smack_known_web.smk_known ||
3805+
c == *smack_known_star.smk_known) {
3806+
rc = -EPERM;
3807+
goto free_labelstr;
3808+
}
3809+
}
38033810

38043811
if (!smack_privileged(CAP_MAC_ADMIN)) {
3805-
rc = -EPERM;
3812+
const struct smack_known_list_elem *sklep;
38063813
list_for_each_entry(sklep, &tsp->smk_relabel, list)
3807-
if (sklep->smk_label == skp) {
3808-
rc = 0;
3809-
break;
3810-
}
3811-
if (rc)
3812-
return rc;
3814+
if (strcmp(sklep->smk_label->smk_known, labelstr) == 0)
3815+
goto free_labelstr;
3816+
rc = -EPERM;
38133817
}
38143818

3819+
free_labelstr:
3820+
kfree(labelstr);
3821+
if (rc)
3822+
return -EPERM;
3823+
3824+
skp = smk_import_entry(value, size);
3825+
if (IS_ERR(skp))
3826+
return PTR_ERR(skp);
3827+
38153828
new = prepare_creds();
38163829
if (new == NULL)
38173830
return -ENOMEM;

0 commit comments

Comments
 (0)