Re: [PATCH] xfrm: Duplicate SPI Handling – IPsec-v3 Compliance Concern

[Herbert Xu <herbert@gondor.apana.org.au>]

On Mon, Jun 02, 2025 at 11:49:48PM +0530, Aakash Kumar S wrote:
>
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 341d79ecb5c2..d0b221a4a625 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -2550,7 +2550,6 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
>  	__be32 minspi = htonl(low);
>  	__be32 maxspi = htonl(high);
>  	__be32 newspi = 0;
> -	u32 mark = x->mark.v & x->mark.m;
>  
>  	spin_lock_bh(&x->lock);
>  	if (x->km.state == XFRM_STATE_DEAD) {
> @@ -2565,7 +2564,7 @@ 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);
> +		x0 = xfrm_state_lookup_byspi(net, minspi, x->props.family);
>  		if (x0) {
>  			NL_SET_ERR_MSG(extack, "Requested SPI is already in use");
>  			xfrm_state_put(x0);
> @@ -2576,7 +2575,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
>  		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_byspi(net, htonl(spi), x->props.family);
>  			if (x0 == NULL) {
>  				newspi = htonl(spi);
>  				break;

The patch looks OK to me.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

That function in general has some issues though.  First of all
the SPI search is racy.  We only take the lock and update the
state database after the search.  That means the supposedly unique
SPI may no longer be unique.

The search for an SPI in the range is also prone to DoS attacks
if the SPI range is large and dense at the same time.  That depends
on how user-space constructs the range of course.

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