[PATCH 1/2] smack: fix bug: unprivileged task can create labels

[Casey Schaufler]

On 3/6/2025 2:43 PM, Konstantin Andreev wrote:
> 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 at swemel.ru>
> ---
>  security/smack/smack_lsm.c | 41 +++++++++++++++++++++++++-------------
>  1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 99833168604e..95a614ae4c9c 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3732,8 +3732,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;
> @@ -3744,28 +3744,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 ||

No space before ".smk_known". I can fix this if/when I take the patch.

> +		    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;