Re: [PATCH] smackfs: reject out-of-range IPv4 octets in netlabel writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] smackfs: out-of-bounds IPv4 octet validation bypassed via integer wrap-around
--

commit 13135cde4c7fdf9b39cec7b05019b4e50745e1ad
Author: Hongjian Dai <daihongjian@kylinsec.com.cn>

smackfs: reject out-of-range IPv4 octets in netlabel writes

This commit attempts to fix a misconfiguration vulnerability in
smk_write_net4addr() where out-of-range IPv4 octets were silently wrapped by
sscanf with %hhd. It changes the sscanf format to %u and adds explicit
bounds checking before storing the address.

> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 6e62dcb36f74f..5d8bddffbabd3 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c

[ ... ]

> @@ -1189,17 +1190,26 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
>  		goto free_data_out;
>  	}
>  
> -	rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd/%u %s",
> -		&host[0], &host[1], &host[2], &host[3], &masks, smack);
> +	rc = sscanf(data, "%u.%u.%u.%u/%u %s",
> +		&octet[0], &octet[1], &octet[2], &octet[3], &masks, smack);
>  	if (rc != 6) {
> -		rc = sscanf(data, "%hhd.%hhd.%hhd.%hhd %s",
> -			&host[0], &host[1], &host[2], &host[3], smack);
> +		rc = sscanf(data, "%u.%u.%u.%u %s",
> +			&octet[0], &octet[1], &octet[2], &octet[3], smack);
>  		if (rc != 5) {
>  			rc = -EINVAL;
>  			goto free_out;
>  		}
>  		masks = 32;
>  	}
> +	if (octet[0] > 255 || octet[1] > 255 || octet[2] > 255 ||
> +	    octet[3] > 255) {

[Severity: Medium]
Does this still silently wrap if an input value is >= 2^32?

The kernel's sscanf implementation for %u parses into a 64-bit unsigned long
long and truncates it to a 32-bit unsigned int.

Input values like 4294967296 are silently truncated to values <= 255, which
would completely bypass this new validation check and reproduce the exact
silent-wrapping misconfiguration this patch intends to fix.

> +		rc = -EINVAL;
> +		goto free_out;
> +	}
> +	host[0] = octet[0];
> +	host[1] = octet[1];
> +	host[2] = octet[2];
> +	host[3] = octet[3];
>  	if (masks > BEBITS) {
>  		rc = -EINVAL;
>  		goto free_out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/FFA41AC93858A96A+20260921014555.20351-1-daihongjian@kylinsec.com.cn?part=1