| Message ID | kybele.cve-2025-39797.v1.841378e108594934aa3d8a5ef9232658.series.J.patch-0001.a71d1f22da0b.a9eab9f5d7caaec8@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2025-39797 | expand |
On 2026/09/19 12:27AM, Cengiz Can via kernel-team wrote: > From: Aakash Kumar S <saakashkumar@marvell.com> > > The issue originates when Strongswan initiates an XFRM_MSG_ALLOCSPI > Netlink message, which triggers the kernel function xfrm_alloc_spi(). > This function is expected to ensure uniqueness of the Security Parameter > Index (SPI) for inbound Security Associations (SAs). However, it can > return success even when the requested SPI is already in use, leading > to duplicate SPIs assigned to multiple inbound SAs, differentiated > only by their destination addresses. > > This behavior causes inconsistencies during SPI lookups for inbound packets. > Since the lookup may return an arbitrary SA among those with the same SPI, > packet processing can fail, resulting in packet drops. > > According to RFC 4301 section 4.4.2 , for inbound processing a unicast SA > is uniquely identified by the SPI and optionally protocol. > > Reproducing the Issue Reliably: > To consistently reproduce the problem, restrict the available SPI range in > charon.conf : spi_min = 0x10000000 spi_max = 0x10000002 > This limits the system to only 2 usable SPI values. > Next, create more than 2 Child SA. each using unique pair of src/dst address. > As soon as the 3rd Child SA is initiated, it will be assigned a duplicate > SPI, since the SPI pool is already exhausted. > With a narrow SPI range, the issue is consistently reproducible. > With a broader/default range, it becomes rare and unpredictable. > > Current implementation: > xfrm_spi_hash() lookup function computes hash using daddr, proto, and family. > So if two SAs have the same SPI but different destination addresses, then > they will: > a. Hash into different buckets > b. Be stored in different linked lists (byspi + h) > c. Not be seen in the same hlist_for_each_entry_rcu() iteration. > As a result, the lookup will result in NULL and kernel allows that Duplicate SPI > > Proposed Change: > xfrm_state_lookup_spi_proto() does a truly global search - across all states, > regardless of hash bucket and matches SPI and proto. > > Signed-off-by: Aakash Kumar S <saakashkumar@marvell.com> > Acked-by: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> > (backported from commit 94f39804d891cffe4ce17737d295f3b195bc7299) Missing follow-up cd8ae32e4e46 ("xfrm: xfrm_alloc_spi shouldn't use 0 as SPI"). > [bot_kybele: Adapted to this tree: xfrm_alloc_spi() has no extack param (dropped > NL_SET_ERR_MSG), replaced get_random_u32_inclusive() with low + prandom_u32() % > range, and XFRM_STATE_INSERT() with hlist_add_head_rcu(); the new > xfrm_state_lookup_spi_proto() applied verbatim.] > CVE-2025-39797 > Assisted-by: kybele:claude-opus-4.8 > Signed-off-by: Cengiz Can <cengiz.can@canonical.com> > --- > net/xfrm/xfrm_state.c | 67 ++++++++++++++++++++++++++----------------- > 1 file changed, 41 insertions(+), 26 deletions(-) > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index 25503d069a64..66d53cefa94f 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c > @@ -1306,6 +1306,26 @@ struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi, > } > EXPORT_SYMBOL(xfrm_state_lookup_byspi); > > +static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net, __be32 spi, u8 proto) > +{ > + struct xfrm_state *x; > + unsigned int i; > + > + rcu_read_lock(); > + for (i = 0; i <= net->xfrm.state_hmask; i++) { > + hlist_for_each_entry_rcu(x, &net->xfrm.state_byspi[i], byspi) { > + if (x->id.spi == spi && x->id.proto == proto) { > + if (!xfrm_state_hold_rcu(x)) > + continue; > + rcu_read_unlock(); > + return x; > + } > + } > + } > + rcu_read_unlock(); > + return NULL; > +} > + > static void __xfrm_state_insert(struct xfrm_state *x) > { > struct net *net = xs_net(x); > @@ -2093,10 +2113,8 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) > unsigned int h; > struct xfrm_state *x0; > int err = -ENOENT; > - __be32 minspi = htonl(low); > - __be32 maxspi = htonl(high); > + u32 range = high - low + 1; > __be32 newspi = 0; > - u32 mark = x->mark.v & x->mark.m; > > spin_lock_bh(&x->lock); > if (x->km.state == XFRM_STATE_DEAD) > @@ -2108,33 +2126,30 @@ 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) { > - xfrm_state_put(x0); > + for (h = 0; h < range; h++) { > + u32 spi = (low == high) ? low : low + prandom_u32() % range; > + newspi = htonl(spi); > + > + spin_lock_bh(&net->xfrm.xfrm_state_lock); > + x0 = xfrm_state_lookup_spi_proto(net, newspi, x->id.proto); > + if (!x0) { > + x->id.spi = newspi; > + h = xfrm_spi_hash(net, &x->id.daddr, newspi, x->id.proto, x->props.family); > + hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h); > + spin_unlock_bh(&net->xfrm.xfrm_state_lock); > + err = 0; > goto unlock; > } > - newspi = minspi; > - } else { > - u32 spi = 0; > - for (h = 0; h < high-low+1; h++) { > - spi = low + prandom_u32()%(high-low+1); > - x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family); > - if (x0 == NULL) { > - newspi = htonl(spi); > - break; > - } > - xfrm_state_put(x0); > - } > - } > - if (newspi) { > - spin_lock_bh(&net->xfrm.xfrm_state_lock); > - x->id.spi = newspi; > - h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, x->props.family); > - hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h); > + xfrm_state_put(x0); > spin_unlock_bh(&net->xfrm.xfrm_state_lock); > > - err = 0; > + if (signal_pending(current)) { > + err = -ERESTARTSYS; > + goto unlock; > + } > + > + if (low == high) > + break; > } > > unlock: > -- > 2.53.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 25503d069a64..66d53cefa94f 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1306,6 +1306,26 @@ struct xfrm_state *xfrm_state_lookup_byspi(struct net *net, __be32 spi, } EXPORT_SYMBOL(xfrm_state_lookup_byspi); +static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net, __be32 spi, u8 proto) +{ + struct xfrm_state *x; + unsigned int i; + + rcu_read_lock(); + for (i = 0; i <= net->xfrm.state_hmask; i++) { + hlist_for_each_entry_rcu(x, &net->xfrm.state_byspi[i], byspi) { + if (x->id.spi == spi && x->id.proto == proto) { + if (!xfrm_state_hold_rcu(x)) + continue; + rcu_read_unlock(); + return x; + } + } + } + rcu_read_unlock(); + return NULL; +} + static void __xfrm_state_insert(struct xfrm_state *x) { struct net *net = xs_net(x); @@ -2093,10 +2113,8 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high) unsigned int h; struct xfrm_state *x0; int err = -ENOENT; - __be32 minspi = htonl(low); - __be32 maxspi = htonl(high); + u32 range = high - low + 1; __be32 newspi = 0; - u32 mark = x->mark.v & x->mark.m; spin_lock_bh(&x->lock); if (x->km.state == XFRM_STATE_DEAD) @@ -2108,33 +2126,30 @@ 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) { - xfrm_state_put(x0); + for (h = 0; h < range; h++) { + u32 spi = (low == high) ? low : low + prandom_u32() % range; + newspi = htonl(spi); + + spin_lock_bh(&net->xfrm.xfrm_state_lock); + x0 = xfrm_state_lookup_spi_proto(net, newspi, x->id.proto); + if (!x0) { + x->id.spi = newspi; + h = xfrm_spi_hash(net, &x->id.daddr, newspi, x->id.proto, x->props.family); + hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h); + spin_unlock_bh(&net->xfrm.xfrm_state_lock); + err = 0; goto unlock; } - newspi = minspi; - } else { - u32 spi = 0; - for (h = 0; h < high-low+1; h++) { - spi = low + prandom_u32()%(high-low+1); - x0 = xfrm_state_lookup(net, mark, &x->id.daddr, htonl(spi), x->id.proto, x->props.family); - if (x0 == NULL) { - newspi = htonl(spi); - break; - } - xfrm_state_put(x0); - } - } - if (newspi) { - spin_lock_bh(&net->xfrm.xfrm_state_lock); - x->id.spi = newspi; - h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, x->props.family); - hlist_add_head_rcu(&x->byspi, net->xfrm.state_byspi + h); + xfrm_state_put(x0); spin_unlock_bh(&net->xfrm.xfrm_state_lock); - err = 0; + if (signal_pending(current)) { + err = -ERESTARTSYS; + goto unlock; + } + + if (low == high) + break; } unlock: