netdev - Re: [PATCH] xfrm: Duplicate SPI Handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aFp8_Be2qHTWmyvI@gondor.apana.org.au>
Date: Tue, 24 Jun 2025 18:25:00 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Aakash Kumar S <saakashkumar@...vell.com>
Cc: netdev@...r.kernel.org, steffen.klassert@...unet.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, akamaluddin@...vell.com,
	antony@...nome.org
Subject: Re: [PATCH] xfrm: Duplicate SPI Handling

On Tue, Jun 24, 2025 at 03:45:16PM +0530, Aakash Kumar S wrote:
>
> @@ -2565,18 +2586,12 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
>  	err = -ENOENT;
>  
>  	if (minspi == maxspi) {
> -		x0 = xfrm_state_lookup(net, mark, &x->id.daddr, minspi, x->id.proto, x->props.family);
> -		if (x0) {
> -			NL_SET_ERR_MSG(extack, "Requested SPI is already in use");
> -			xfrm_state_put(x0);
> -			goto unlock;
> -		}
>  		newspi = minspi;
>  	} else {
>  		u32 spi = 0;
>  		for (h = 0; h < high-low+1; h++) {
>  			spi = get_random_u32_inclusive(low, high);
> -			x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family);
> +			x0 = xfrm_state_lookup_spi_proto(net, htonl(spi), x->id.proto);
>  			if (x0 == NULL) {
>  				newspi = htonl(spi);
>  				break;
> @@ -2586,6 +2601,13 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
>  	}
>  	if (newspi) {
>  		spin_lock_bh(&net->xfrm.xfrm_state_lock);
> +		x0 = xfrm_state_lookup_spi_proto(net, newspi, x->id.proto);
> +		if (x0) {
> +			NL_SET_ERR_MSG(extack, "Requested SPI is already in use");
> +			xfrm_state_put(x0);
> +			goto unlock;
> +		}
> +

There is a quality of implementation problem here.  The double checks
are racy and may result in an "already in use" error even though the
specified SPI range is not fully used up.

Ideally the range check should be conducted under lock, or there should
be a retry if the insertion failed for an SPI range request.  I think
a retry would be the easiest with an additional check for signals
interrupting the request.

Cheers,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt