[REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Yan Yan]

Hi all,

I am writing because unfortunately commit: 94f39804d891 ("xfrm:
Duplicate SPI Handling") has caused a regression in the Android OS, so
we would like to gain some context to help determine next steps. The
issue is caused by the new requirement for global SPI uniqueness
during allocation. Based on our review of RFC 4301 and the previous
review history, we would like to highlight a few concerns:

1. Regression on Android:
This change breaks production behavior in Android, which uses
XFRM_MSG_ALLOCSPI to create larval SAs for both directions. Since RFC
4301 allows duplicate outbound SPIs, and larval SAs are often created
before direction or selectors are finalized, the allocator must remain
permissive (at least in our current design).
This also aligns with a concern Herbert Xu raised during the initial
review regarding compatibility:
>    "It's also dangerous to unilaterally do this since existing deployments
>    could rely on the old behaviour. You'd need to add a toggle for
>    compatibility."

2. Inbound SPI uniqueness should not be a hard requirement:
The justification for enforcing global SPI uniqueness often cites the
statement in RFC 4301, Section 4.1, that for unicast traffic, the SPI
"by itself suffices to specify an SA." However, we don’t think this
means inbound SPI uniqueness is a hard requirement because of the two
following reasons:

– Another statement implies that SPI uniqueness is just an
implementation choice:
>    "Each entry in the SA Database (SAD) MUST
>    indicate whether the SA lookup makes use of the destination IP address, or the
>    destination and source IP addresses, in addition to the SPI."

– There is a "Longest Match" mandate which makes SPI uniqueness unnecessary:
>    "For each inbound, IPsec-protected packet, an implementation MUST
>    conduct its search of the SAD such that it finds the entry that
>    matches the 'longest' SA identifier. In this context, if two or more
>    SAD entries match based on the SPI value, then the entry that also
>    matches based on destination address... is the 'longest' match."

3. Further clarification on the specific problem being addressed would
be helpful. The "real-world" problem this commit intends to fix
remains unclear. The patch mentions:
>    "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."

However, Linux kernel lookups using the triplet (SPI, Protocol, Daddr)
are deterministic. The lookup will not return an "arbitrary" SA
because the destination address is used to disambiguate the state.

As Antony suggested, this change may cater to SPI-only hardware
indexing. If that is the case, we are concerned about applying such
hardware-specific limits to the software stack, especially if the
behavior is not opt-in, as it appears to require an overly-narrow
reading of the RFC 4301.

Given these concerns, would it be possible to discuss a revert or,
alternatively, could further context be provided regarding the
specific real-world problem this commit was intended to address? Once
the underlying issue is clearly defined, we can work together to find
a backward-compatible solution that satisfies all requirements.

Review threads are attached for easy reference:
https://lore.kernel.org/netdev/aDQhZ_ikHEt_pLn_@gondor.apana.org.au/T/#r45c1786651ce5af730f757aca7438474d494a323
https://lore.kernel.org/netdev/20250616100621.837472-1-saakashkumar@marvell.com/T/#u

Best regards,

Yan (on behalf of the Android IPsec team)

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Steffen Klassert]

On Wed, Jan 28, 2026 at 04:43:06PM +0800, Yan Yan wrote:
> Hi all,
> 
> I am writing because unfortunately commit: 94f39804d891 ("xfrm:
> Duplicate SPI Handling") has caused a regression in the Android OS, so
> we would like to gain some context to help determine next steps. The
> issue is caused by the new requirement for global SPI uniqueness
> during allocation. Based on our review of RFC 4301 and the previous
> review history, we would like to highlight a few concerns:
> 
> 1. Regression on Android:
> This change breaks production behavior in Android, which uses
> XFRM_MSG_ALLOCSPI to create larval SAs for both directions. Since RFC
> 4301 allows duplicate outbound SPIs, and larval SAs are often created
> before direction or selectors are finalized, the allocator must remain
> permissive (at least in our current design).
> This also aligns with a concern Herbert Xu raised during the initial
> review regarding compatibility:
> >    "It's also dangerous to unilaterally do this since existing deployments
> >    could rely on the old behaviour. You'd need to add a toggle for
> >    compatibility."

Indeed, it was...

> 2. Inbound SPI uniqueness should not be a hard requirement:
> The justification for enforcing global SPI uniqueness often cites the
> statement in RFC 4301, Section 4.1, that for unicast traffic, the SPI
> "by itself suffices to specify an SA." However, we don’t think this
> means inbound SPI uniqueness is a hard requirement because of the two
> following reasons:
> 
> – Another statement implies that SPI uniqueness is just an
> implementation choice:
> >    "Each entry in the SA Database (SAD) MUST
> >    indicate whether the SA lookup makes use of the destination IP address, or the
> >    destination and source IP addresses, in addition to the SPI."

When reading RFC 4301, I'd interpret this as 'The SA
must indicate whether it is unicast or multicast'.

> 
> – There is a "Longest Match" mandate which makes SPI uniqueness unnecessary:
> >    "For each inbound, IPsec-protected packet, an implementation MUST
> >    conduct its search of the SAD such that it finds the entry that
> >    matches the 'longest' SA identifier. In this context, if two or more
> >    SAD entries match based on the SPI value, then the entry that also
> >    matches based on destination address... is the 'longest' match."

This is meant for multicast SAs, we don't support multicast SAs.

> 3. Further clarification on the specific problem being addressed would
> be helpful. The "real-world" problem this commit intends to fix
> remains unclear. The patch mentions:
> >    "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."
> 
> However, Linux kernel lookups using the triplet (SPI, Protocol, Daddr)
> are deterministic. The lookup will not return an "arbitrary" SA
> because the destination address is used to disambiguate the state.

That was how RFC 2401 did it:

A security association is uniquely identified by a triple consisting
of a Security Parameter Index (SPI), an IP Destination Address, and a
security protocol (AH or ESP) identifier.

That was obsoleted by RFC RFC 4301 saying the SPI alone identifies
the SA for unicast traffic.

Unfortunately our implementation is somewhat between RFC 2401
and RFC 4301. There was no clear cut to support both versions.

Maybe it is time to do this now and implement RFC 4301
with a toggle to fallback to RFC 2401. This would avoid
these kind of problems in the future, at least.

> As Antony suggested, this change may cater to SPI-only hardware
> indexing. If that is the case, we are concerned about applying such
> hardware-specific limits to the software stack, especially if the
> behavior is not opt-in, as it appears to require an overly-narrow
> reading of the RFC 4301.
> 
> Given these concerns, would it be possible to discuss a revert or,
> alternatively, could further context be provided regarding the
> specific real-world problem this commit was intended to address? Once
> the underlying issue is clearly defined, we can work together to find
> a backward-compatible solution that satisfies all requirements.

Here we end up with the question about the default behaviour,
should the default be RFC 4301 or RFC 2401?

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Antony Antony]

Hi Yan,

On Wed, Jan 28, 2026 at 16:43:06 +0800, Yan Yan wrote:
> Hi all,
> 
> I am writing because unfortunately commit: 94f39804d891 ("xfrm:
> Duplicate SPI Handling") has caused a regression in the Android OS, so
> we would like to gain some context to help determine next steps. The
> issue is caused by the new requirement for global SPI uniqueness
> during allocation. Based on our review of RFC 4301 and the previous
> review history, we would like to highlight a few concerns:
> 
> 1. Regression on Android:
> This change breaks production behavior in Android, which uses
> XFRM_MSG_ALLOCSPI to create larval SAs for both directions. Since RFC
> 4301 allows duplicate outbound SPIs, and larval SAs are often created

To clarify, how are larval outbound SAs created in Android?
Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed. However, I guess there is no user API for managing these acquire states. Only the kernel can create them and handle expiration or deletion via SA updates with acq_req?

A user API (UAPI) for creating and deleting acquire states might be a possible solution.

I haven’t been able to consistently reproduce the issue Marvell reported,
but I suspect the bug could also affect outbound SAs with non-zero SPIs.
Also when one peer is behind NAT. 

I wonder wouldn't duplicate SPI when behind NAT cause issues for output SAs? 
Because the triplet is SPI, Protocol, Daddr. There is  no dport in it.

> before direction or selectors are finalized, the allocator must remain
> permissive (at least in our current design).
> This also aligns with a concern Herbert Xu raised during the initial
> review regarding compatibility:
> >    "It's also dangerous to unilaterally do this since existing deployments
> >    could rely on the old behaviour. You'd need to add a toggle for
> >    compatibility."
> 
> 2. Inbound SPI uniqueness should not be a hard requirement:
> The justification for enforcing global SPI uniqueness often cites the
> statement in RFC 4301, Section 4.1, that for unicast traffic, the SPI
> "by itself suffices to specify an SA." However, we don’t think this
> means inbound SPI uniqueness is a hard requirement because of the two
> following reasons:
> 
> – Another statement implies that SPI uniqueness is just an
> implementation choice:
> >    "Each entry in the SA Database (SAD) MUST
> >    indicate whether the SA lookup makes use of the destination IP address, or the
> >    destination and source IP addresses, in addition to the SPI."
> 
> – There is a "Longest Match" mandate which makes SPI uniqueness unnecessary:
> >    "For each inbound, IPsec-protected packet, an implementation MUST
> >    conduct its search of the SAD such that it finds the entry that
> >    matches the 'longest' SA identifier. In this context, if two or more
> >    SAD entries match based on the SPI value, then the entry that also
> >    matches based on destination address... is the 'longest' match."
> 
> 3. Further clarification on the specific problem being addressed would
> be helpful. The "real-world" problem this commit intends to fix
> remains unclear. The patch mentions:
> >    "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."
> 
> However, Linux kernel lookups using the triplet (SPI, Protocol, Daddr)
> are deterministic. The lookup will not return an "arbitrary" SA
> because the destination address is used to disambiguate the state.
> 
> As Antony suggested, this change may cater to SPI-only hardware
> indexing. If that is the case, we are concerned about applying such
> hardware-specific limits to the software stack, especially if the
> behavior is not opt-in, as it appears to require an overly-narrow
> reading of the RFC 4301. 

I agree with your suggestion that making the behavior opt-in. 
I would prefer the Default : to allow duplicate.

> Given these concerns, would it be possible to discuss a revert or,
> alternatively, could further context be provided regarding the
> specific real-world problem this commit was intended to address? Once
> the underlying issue is clearly defined, we can work together to find
> a backward-compatible solution that satisfies all requirements.
> 
> Review threads are attached for easy reference:
> https://lore.kernel.org/netdev/aDQhZ_ikHEt_pLn_@gondor.apana.org.au/T/#r45c1786651ce5af730f757aca7438474d494a323
> https://lore.kernel.org/netdev/20250616100621.837472-1-saakashkumar@marvell.com/T/#u

-antony

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Yan Yan]

Thank you all for the reply! Please see my response inline below

> This is meant for multicast SAs, we don't support multicast SAs.
I reread the RFC and that makes sense to me. Thanks for the explanation.

> To clarify, how are larval outbound SAs created in Android?
> Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed.

In Android, larval outbound SAs are created with non-zero SPIs because
Android userspace uses XFRM_MSG_ALLOCSPI to manually reserve a unique
value before the SA is fully finalized. These are not kernel-generated
"acquire" states.

Regarding the default behavior:

I'm fine with defaulting to RFC 4301 for the upstream kernel to align
with the modern standard. As long as a toggle exists to fallback to
the legacy RFC 2401 behavior, Android’s compatibility requirements are
met.




On Mon, Feb 2, 2026 at 10:27 PM Antony Antony <antony.antony@secunet.com> wrote:
>
> Hi Yan,
>
> On Wed, Jan 28, 2026 at 16:43:06 +0800, Yan Yan wrote:
> > Hi all,
> >
> > I am writing because unfortunately commit: 94f39804d891 ("xfrm:
> > Duplicate SPI Handling") has caused a regression in the Android OS, so
> > we would like to gain some context to help determine next steps. The
> > issue is caused by the new requirement for global SPI uniqueness
> > during allocation. Based on our review of RFC 4301 and the previous
> > review history, we would like to highlight a few concerns:
> >
> > 1. Regression on Android:
> > This change breaks production behavior in Android, which uses
> > XFRM_MSG_ALLOCSPI to create larval SAs for both directions. Since RFC
> > 4301 allows duplicate outbound SPIs, and larval SAs are often created
>
> To clarify, how are larval outbound SAs created in Android?
> Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed. However, I guess there is no user API for managing these acquire states. Only the kernel can create them and handle expiration or deletion via SA updates with acq_req?
>
> A user API (UAPI) for creating and deleting acquire states might be a possible solution.
>
> I haven’t been able to consistently reproduce the issue Marvell reported,
> but I suspect the bug could also affect outbound SAs with non-zero SPIs.
> Also when one peer is behind NAT.
>
> I wonder wouldn't duplicate SPI when behind NAT cause issues for output SAs?
> Because the triplet is SPI, Protocol, Daddr. There is  no dport in it.
>
> > before direction or selectors are finalized, the allocator must remain
> > permissive (at least in our current design).
> > This also aligns with a concern Herbert Xu raised during the initial
> > review regarding compatibility:
> > >    "It's also dangerous to unilaterally do this since existing deployments
> > >    could rely on the old behaviour. You'd need to add a toggle for
> > >    compatibility."
> >
> > 2. Inbound SPI uniqueness should not be a hard requirement:
> > The justification for enforcing global SPI uniqueness often cites the
> > statement in RFC 4301, Section 4.1, that for unicast traffic, the SPI
> > "by itself suffices to specify an SA." However, we don’t think this
> > means inbound SPI uniqueness is a hard requirement because of the two
> > following reasons:
> >
> > – Another statement implies that SPI uniqueness is just an
> > implementation choice:
> > >    "Each entry in the SA Database (SAD) MUST
> > >    indicate whether the SA lookup makes use of the destination IP address, or the
> > >    destination and source IP addresses, in addition to the SPI."
> >
> > – There is a "Longest Match" mandate which makes SPI uniqueness unnecessary:
> > >    "For each inbound, IPsec-protected packet, an implementation MUST
> > >    conduct its search of the SAD such that it finds the entry that
> > >    matches the 'longest' SA identifier. In this context, if two or more
> > >    SAD entries match based on the SPI value, then the entry that also
> > >    matches based on destination address... is the 'longest' match."
> >
> > 3. Further clarification on the specific problem being addressed would
> > be helpful. The "real-world" problem this commit intends to fix
> > remains unclear. The patch mentions:
> > >    "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."
> >
> > However, Linux kernel lookups using the triplet (SPI, Protocol, Daddr)
> > are deterministic. The lookup will not return an "arbitrary" SA
> > because the destination address is used to disambiguate the state.
> >
> > As Antony suggested, this change may cater to SPI-only hardware
> > indexing. If that is the case, we are concerned about applying such
> > hardware-specific limits to the software stack, especially if the
> > behavior is not opt-in, as it appears to require an overly-narrow
> > reading of the RFC 4301.
>
> I agree with your suggestion that making the behavior opt-in.
> I would prefer the Default : to allow duplicate.
>
> > Given these concerns, would it be possible to discuss a revert or,
> > alternatively, could further context be provided regarding the
> > specific real-world problem this commit was intended to address? Once
> > the underlying issue is clearly defined, we can work together to find
> > a backward-compatible solution that satisfies all requirements.
> >
> > Review threads are attached for easy reference:
> > https://lore.kernel.org/netdev/aDQhZ_ikHEt_pLn_@gondor.apana.org.au/T/#r45c1786651ce5af730f757aca7438474d494a323
> > https://lore.kernel.org/netdev/20250616100621.837472-1-saakashkumar@marvell.com/T/#u
>
> -antony



--
--
Best,
Yan

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Nathan Harold]

Unfortunately, Android's requirements significantly limit our ability
to implement compatibility fixes between the OS and kernel due to
interface freezes and cross-version compatibility requirements, so
thank you for taking us into consideration! Generally, without a
userspace mechanism to opt in to new behavior though, it's tough. For
now, the Android kernel is just reverting these patches, but that's
not a long-term strategy.

One approach, which seems generally sound to me for this specific
issue (and perhaps more broadly), would be to leverage the relatively
new XFRM_SA_DIR attribute to distinguish between legacy behavior and
stricter 4301 compliance. Much of the optimization is
direction-specific, so we can infer that an implementation using the
newer API and opting in to direction-specific behavior wants 4301/new
feature support--also, Android doesn't yet use that attribute :-) .
Or, as Steffen suggested, there could be a new toggle that indicates a
standards-based target behavior (like 4301), which sounds great if the
SA_DIR flag is deemed insufficient. Thoughts?

-Nathan


On Sun, Feb 8, 2026 at 7:03 PM Yan Yan <evitayan@google.com> wrote:
>
> Thank you all for the reply! Please see my response inline below
>
> > This is meant for multicast SAs, we don't support multicast SAs.
> I reread the RFC and that makes sense to me. Thanks for the explanation.
>
> > To clarify, how are larval outbound SAs created in Android?
> > Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed.
>
> In Android, larval outbound SAs are created with non-zero SPIs because
> Android userspace uses XFRM_MSG_ALLOCSPI to manually reserve a unique
> value before the SA is fully finalized. These are not kernel-generated
> "acquire" states.
>
> Regarding the default behavior:
>
> I'm fine with defaulting to RFC 4301 for the upstream kernel to align
> with the modern standard. As long as a toggle exists to fallback to
> the legacy RFC 2401 behavior, Android’s compatibility requirements are
> met.
>
>
>
>
> On Mon, Feb 2, 2026 at 10:27 PM Antony Antony <antony.antony@secunet.com> wrote:
> >
> > Hi Yan,
> >
> > On Wed, Jan 28, 2026 at 16:43:06 +0800, Yan Yan wrote:
> > > Hi all,
> > >
> > > I am writing because unfortunately commit: 94f39804d891 ("xfrm:
> > > Duplicate SPI Handling") has caused a regression in the Android OS, so
> > > we would like to gain some context to help determine next steps. The
> > > issue is caused by the new requirement for global SPI uniqueness
> > > during allocation. Based on our review of RFC 4301 and the previous
> > > review history, we would like to highlight a few concerns:
> > >
> > > 1. Regression on Android:
> > > This change breaks production behavior in Android, which uses
> > > XFRM_MSG_ALLOCSPI to create larval SAs for both directions. Since RFC
> > > 4301 allows duplicate outbound SPIs, and larval SAs are often created
> >
> > To clarify, how are larval outbound SAs created in Android?
> > Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed. However, I guess there is no user API for managing these acquire states. Only the kernel can create them and handle expiration or deletion via SA updates with acq_req?
> >
> > A user API (UAPI) for creating and deleting acquire states might be a possible solution.
> >
> > I haven’t been able to consistently reproduce the issue Marvell reported,
> > but I suspect the bug could also affect outbound SAs with non-zero SPIs.
> > Also when one peer is behind NAT.
> >
> > I wonder wouldn't duplicate SPI when behind NAT cause issues for output SAs?
> > Because the triplet is SPI, Protocol, Daddr. There is  no dport in it.
> >
> > > before direction or selectors are finalized, the allocator must remain
> > > permissive (at least in our current design).
> > > This also aligns with a concern Herbert Xu raised during the initial
> > > review regarding compatibility:
> > > >    "It's also dangerous to unilaterally do this since existing deployments
> > > >    could rely on the old behaviour. You'd need to add a toggle for
> > > >    compatibility."
> > >
> > > 2. Inbound SPI uniqueness should not be a hard requirement:
> > > The justification for enforcing global SPI uniqueness often cites the
> > > statement in RFC 4301, Section 4.1, that for unicast traffic, the SPI
> > > "by itself suffices to specify an SA." However, we don’t think this
> > > means inbound SPI uniqueness is a hard requirement because of the two
> > > following reasons:
> > >
> > > – Another statement implies that SPI uniqueness is just an
> > > implementation choice:
> > > >    "Each entry in the SA Database (SAD) MUST
> > > >    indicate whether the SA lookup makes use of the destination IP address, or the
> > > >    destination and source IP addresses, in addition to the SPI."
> > >
> > > – There is a "Longest Match" mandate which makes SPI uniqueness unnecessary:
> > > >    "For each inbound, IPsec-protected packet, an implementation MUST
> > > >    conduct its search of the SAD such that it finds the entry that
> > > >    matches the 'longest' SA identifier. In this context, if two or more
> > > >    SAD entries match based on the SPI value, then the entry that also
> > > >    matches based on destination address... is the 'longest' match."
> > >
> > > 3. Further clarification on the specific problem being addressed would
> > > be helpful. The "real-world" problem this commit intends to fix
> > > remains unclear. The patch mentions:
> > > >    "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."
> > >
> > > However, Linux kernel lookups using the triplet (SPI, Protocol, Daddr)
> > > are deterministic. The lookup will not return an "arbitrary" SA
> > > because the destination address is used to disambiguate the state.
> > >
> > > As Antony suggested, this change may cater to SPI-only hardware
> > > indexing. If that is the case, we are concerned about applying such
> > > hardware-specific limits to the software stack, especially if the
> > > behavior is not opt-in, as it appears to require an overly-narrow
> > > reading of the RFC 4301.
> >
> > I agree with your suggestion that making the behavior opt-in.
> > I would prefer the Default : to allow duplicate.
> >
> > > Given these concerns, would it be possible to discuss a revert or,
> > > alternatively, could further context be provided regarding the
> > > specific real-world problem this commit was intended to address? Once
> > > the underlying issue is clearly defined, we can work together to find
> > > a backward-compatible solution that satisfies all requirements.
> > >
> > > Review threads are attached for easy reference:
> > > https://lore.kernel.org/netdev/aDQhZ_ikHEt_pLn_@gondor.apana.org.au/T/#r45c1786651ce5af730f757aca7438474d494a323
> > > https://lore.kernel.org/netdev/20250616100621.837472-1-saakashkumar@marvell.com/T/#u
> >
> > -antony
>
>
>
> --
> --
> Best,
> Yan

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Tobias Brunner]

Hi Yan,

>> To clarify, how are larval outbound SAs created in Android?
>> Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed.
> 
> In Android, larval outbound SAs are created with non-zero SPIs because
> Android userspace uses XFRM_MSG_ALLOCSPI to manually reserve a unique
> value before the SA is fully finalized. These are not kernel-generated
> "acquire" states.

Could you clarify what exactly the reasoning is behind this?  Do you use
this for every outbound SA or just for special use cases?  Do you set
the same SPI for in- and outbound SA?  (If so, what happens later when
the SPI of the outbound SA is received from the peer?)  Or do you use
ALLOCSPI once you already know the peer's SPI?  (And only run into the
issue if both allocate the same SPI by chance.)  If so, why would you
use ALLOCSPI and not just install the outbound SA?  Is it to avoid
differences for in- and outbound SAs (ALLOCSPI+UPDSA vs. NEWSA)?

Regards,
Tobias

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Yan Yan]

Hi Tobias,

Regarding your questions:

For every inbound SA, we allocate SPIs before negotiation. For
outbound SAs, we allocate SPIs once requested by the peer. We only
require the (SPI, destination address) combo to be unique. Thus, we
may have an inbound and outbound SA sharing an SPI with different
destinations, or multiple outbound SAs to different peers sharing an
SPI.

> If so, why would you use ALLOCSPI and not just install the outbound SA? Is it to avoid differences for in- and outbound SAs (ALLOCSPI+UPDSA vs. NEWSA)?"

Exactly—it is primarily for code symmetry. By using ALLOCSPI + UPDSA
for both directions, we maintain a consistent larval lifecycle and
make it easier to maintain.


On Tue, Feb 10, 2026 at 6:17 PM Tobias Brunner <tobias@strongswan.org> wrote:
>
> Hi Yan,
>
> >> To clarify, how are larval outbound SAs created in Android?
> >> Are they created with zero or non-zero SPIs? Multiple zero SPIs (acquire states) are allowed.
> >
> > In Android, larval outbound SAs are created with non-zero SPIs because
> > Android userspace uses XFRM_MSG_ALLOCSPI to manually reserve a unique
> > value before the SA is fully finalized. These are not kernel-generated
> > "acquire" states.
>
> Could you clarify what exactly the reasoning is behind this?  Do you use
> this for every outbound SA or just for special use cases?  Do you set
> the same SPI for in- and outbound SA?  (If so, what happens later when
> the SPI of the outbound SA is received from the peer?)  Or do you use
> ALLOCSPI once you already know the peer's SPI?  (And only run into the
> issue if both allocate the same SPI by chance.)  If so, why would you
> use ALLOCSPI and not just install the outbound SA?  Is it to avoid
> differences for in- and outbound SAs (ALLOCSPI+UPDSA vs. NEWSA)?
>
> Regards,
> Tobias
>


-- 
--
Best,
Yan

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Tobias Brunner]

Hi Yan,

> For every inbound SA, we allocate SPIs before negotiation. For
> outbound SAs, we allocate SPIs once requested by the peer. We only
> require the (SPI, destination address) combo to be unique. Thus, we
> may have an inbound and outbound SA sharing an SPI with different
> destinations, or multiple outbound SAs to different peers sharing an
> SPI.

That should still be allowed when using the intended APIs (i.e. ALLOCSPI
for the inbound and NEWSA for the outbound SA).  ALLOCSPI might enforce
a unique SPI without considering the address, as that's intended for
local, inbound SAs, where the kernel has full control (looking at the
the patch, it's certainly not ideal, as it goes through all installed
SAs to find a duplicate and it prevents an inbound SPI that matches an
existing outbound SPI - I guess that could be resolved by using separate
tables for in- and outbound SAs).  But that must not prevent installing
outbound SAs with the same SPI to another peer using NEWSA, which still
uses a hash that includes the destination address (that must always be
the case because peers are free to allocate whatever SPI they want).

>> If so, why would you use ALLOCSPI and not just install the outbound SA? Is it to avoid differences for in- and outbound SAs (ALLOCSPI+UPDSA vs. NEWSA)?"
> 
> Exactly—it is primarily for code symmetry. By using ALLOCSPI + UPDSA
> for both directions, we maintain a consistent larval lifecycle and
> make it easier to maintain.

In my opinion, you are using the API incorrectly.  ALLOCSPI is intended
to allocate a free local SPI for an inbound SA.  That is, reserve it
before and while the details of the SA are negotiated with the peer
using IKE.  This step isn't necessary for outbound SAs and forcing such
an allocation, after all the details are known, to the responder's SPI
(which I assume you do via min/max option) doesn't feel right.  I also
don't think there are any benefits in that "consistent larval lifecycle"
(if you found any, please let us know).  And the difference between
UPDSA and NEWSA is the nlmsg_type (there are some attributes that are
different for in- and outbound SAs, especially if you set the direction
in newer kernels, but that's the case regardless of the message type).

By the way, are you using the min/max option for inbound SAs as well,
with an SPI generated in userland?  That would seem like a violation of
the intention of the API as well (i.e. letting the kernel control the
local SPIs).

As the XFRM API basically mirrors PF_KEYv2 here, you can find more about
the two ways to install SAs in RFC 2367 (SADB_GETSPI/UPDATE vs. SADB_ADD).

Regards,
Tobias

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Nathan Harold]

> That should still be allowed when using the intended APIs (i.e. ALLOCSPI
> for the inbound and NEWSA for the outbound SA).  ALLOCSPI might enforce
> a unique SPI without considering the address, as that's intended for
> local, inbound SAs, where the kernel has full control (looking at the
> the patch, it's certainly not ideal, as it goes through all installed
> SAs to find a duplicate and it prevents an inbound SPI that matches an
> existing outbound SPI - I guess that could be resolved by using separate
> tables for in- and outbound SAs).  But that must not prevent installing
> outbound SAs with the same SPI to another peer using NEWSA, which still
> uses a hash that includes the destination address (that must always be
> the case because peers are free to allocate whatever SPI they want).

Agreed that there are some unfortunate limitations with the current
patch. Keying off the inclusion of XFRM_SA_DIR would resolve the issue
you noted (conflating inbound and outbound SPIs) and function as an
opt-in for this enforcement. Whatever the mechanism though, the new
behavior should be opt-in rather than opt-out in order to maintain
backwards compatibility.

> In my opinion, you are using the API incorrectly...  I also
> don't think there are any benefits in that "consistent larval lifecycle"
> (if you found any, please let us know).

The Android architecture is multi-tenant and allows userspace apps to
establish SAs. At the time we designed it, this felt like the cleanest
way to facilitate leak-free resource management because the chain of
associations between kernel resources could be symmetrical (and
managing them was already quite complicated). Mea culpa (Nathan). But,
correctly or not, it has/had worked for many years.

> By the way, are you using the min/max option for inbound SAs as well,
> with an SPI generated in userland?  That would seem like a violation of
> the intention of the API as well (i.e. letting the kernel control the
> local SPIs).

We provide following two Android APIs for app developers:
https://developer.android.com/reference/android/net/IpSecManager#allocateSecurityParameterIndex(java.net.InetAddress)
https://developer.android.com/reference/android/net/IpSecManager#allocateSecurityParameterIndex(java.net.InetAddress,%20int)

Indeed, allocateSecurityParameterIndex is direction-agnostic; both
overloads are implemented internally by including the min and max
values in ALLOCSPI. For the variant where app developers provide a
specific SPI (which is also useful in testing), Android simply sets
both the min and max parameters to that exact value. Our understanding
of #xfrm_alloc_spi  is that min/max are required for ALLOCSPI, and
otherwise ENOENT will be returned.

Note that we also use the DADDR as a mandatory part of the tuple
because of the issue mentioned above: SPIs are only unique in
conjunction with a DADDR, regardless of direction, and accordingly,
that’s how Android is expecting the uniqueness requirement be
enforced. In this way, 5 duplicate SPIs can be used on 5 unique IP
addresses on the same machine; therefore, a strict "SPI only"
interpretation for ALLOCSPI (or SPI handling in general) is curious.
We feel that ALLOCSPI should really enforce the same uniqueness
requirements as the SAD.

Best,

Nathan and Yan




-Nathan


On Wed, Feb 18, 2026 at 12:42 AM Tobias Brunner <tobias@strongswan.org> wrote:
>
> Hi Yan,
>
> > For every inbound SA, we allocate SPIs before negotiation. For
> > outbound SAs, we allocate SPIs once requested by the peer. We only
> > require the (SPI, destination address) combo to be unique. Thus, we
> > may have an inbound and outbound SA sharing an SPI with different
> > destinations, or multiple outbound SAs to different peers sharing an
> > SPI.
>
> That should still be allowed when using the intended APIs (i.e. ALLOCSPI
> for the inbound and NEWSA for the outbound SA).  ALLOCSPI might enforce
> a unique SPI without considering the address, as that's intended for
> local, inbound SAs, where the kernel has full control (looking at the
> the patch, it's certainly not ideal, as it goes through all installed
> SAs to find a duplicate and it prevents an inbound SPI that matches an
> existing outbound SPI - I guess that could be resolved by using separate
> tables for in- and outbound SAs).  But that must not prevent installing
> outbound SAs with the same SPI to another peer using NEWSA, which still
> uses a hash that includes the destination address (that must always be
> the case because peers are free to allocate whatever SPI they want).
>
> >> If so, why would you use ALLOCSPI and not just install the outbound SA? Is it to avoid differences for in- and outbound SAs (ALLOCSPI+UPDSA vs. NEWSA)?"
> >
> > Exactly—it is primarily for code symmetry. By using ALLOCSPI + UPDSA
> > for both directions, we maintain a consistent larval lifecycle and
> > make it easier to maintain.
>
> In my opinion, you are using the API incorrectly.  ALLOCSPI is intended
> to allocate a free local SPI for an inbound SA.  That is, reserve it
> before and while the details of the SA are negotiated with the peer
> using IKE.  This step isn't necessary for outbound SAs and forcing such
> an allocation, after all the details are known, to the responder's SPI
> (which I assume you do via min/max option) doesn't feel right.  I also
> don't think there are any benefits in that "consistent larval lifecycle"
> (if you found any, please let us know).  And the difference between
> UPDSA and NEWSA is the nlmsg_type (there are some attributes that are
> different for in- and outbound SAs, especially if you set the direction
> in newer kernels, but that's the case regardless of the message type).
>
> By the way, are you using the min/max option for inbound SAs as well,
> with an SPI generated in userland?  That would seem like a violation of
> the intention of the API as well (i.e. letting the kernel control the
> local SPIs).
>
> As the XFRM API basically mirrors PF_KEYv2 here, you can find more about
> the two ways to install SAs in RFC 2367 (SADB_GETSPI/UPDATE vs. SADB_ADD).
>
> Regards,
> Tobias
>

Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Antony Antony]

[-- Attachment #1: Type: text/plain, Size: 8761 bytes --]

Hi,
I looked into this. I feel a simple solution is use x->dir as Nathan
proposed.
When dir is not set we get  pre
commit 94f39804d891 ("xfrm: Duplicate SPI Handling") behaviour.

When XFRM_SA_DIR is set alloc_spi() returns per direction unique spi. 

Another benfit is, this would also keep PF_KEY use case as it was before that comit.
Here is simple RFC patch attached.

How does this look?

strongswan 6.0.0, from Dec 2024, sets x->dir. 
Aakash would this work for for marvell? 

regads,
-antony

On Fri, Mar 27, 2026 at 17:05:13 -0700, Yan Yan wrote:
>    Hi all,
>    I wanted to send a friendly ping to see if we are aligning on making
>    the strict global SPI uniqueness requirement optional, perhaps via a
>    toggle or by leveraging the XFRM_SA_DIR attribute as previously
>    discussed.
>    Are there any other questions or concerns regarding this approach, or
>    anything else we should clarify to ensure backward compatibility while
>    meeting the needs of modern standards?
>    Best,
>    Yan
> 
>    On Tue, Feb 24, 2026 at 3:53 PM Nathan Harold <[1]nharold@google.com>
>    wrote:
> 
>      > That should still be allowed when using the intended APIs (i.e.
>      ALLOCSPI
>      > for the inbound and NEWSA for the outbound SA).  ALLOCSPI might
>      enforce
>      > a unique SPI without considering the address, as that's intended
>      for
>      > local, inbound SAs, where the kernel has full control (looking at
>      the
>      > the patch, it's certainly not ideal, as it goes through all
>      installed
>      > SAs to find a duplicate and it prevents an inbound SPI that
>      matches an
>      > existing outbound SPI - I guess that could be resolved by using
>      separate
>      > tables for in- and outbound SAs).  But that must not prevent
>      installing
>      > outbound SAs with the same SPI to another peer using NEWSA, which
>      still
>      > uses a hash that includes the destination address (that must
>      always be
>      > the case because peers are free to allocate whatever SPI they
>      want).
>      Agreed that there are some unfortunate limitations with the current
>      patch. Keying off the inclusion of XFRM_SA_DIR would resolve the
>      issue
>      you noted (conflating inbound and outbound SPIs) and function as an
>      opt-in for this enforcement. Whatever the mechanism though, the new
>      behavior should be opt-in rather than opt-out in order to maintain
>      backwards compatibility.
>      > In my opinion, you are using the API incorrectly...  I also
>      > don't think there are any benefits in that "consistent larval
>      lifecycle"
>      > (if you found any, please let us know).
>      The Android architecture is multi-tenant and allows userspace apps
>      to
>      establish SAs. At the time we designed it, this felt like the
>      cleanest
>      way to facilitate leak-free resource management because the chain of
>      associations between kernel resources could be symmetrical (and
>      managing them was already quite complicated). Mea culpa (Nathan).
>      But,
>      correctly or not, it has/had worked for many years.
>      > By the way, are you using the min/max option for inbound SAs as
>      well,
>      > with an SPI generated in userland?  That would seem like a
>      violation of
>      > the intention of the API as well (i.e. letting the kernel control
>      the
>      > local SPIs).
>      We provide following two Android APIs for app developers:
>      [2]https://developer.android.com/reference/android/net/IpSecManager#
>      allocateSecurityParameterIndex(java.net.InetAddress)
>      [3]https://developer.android.com/reference/android/net/IpSecManager#
>      allocateSecurityParameterIndex(java.net.InetAddress,%20int)
>      Indeed, allocateSecurityParameterIndex is direction-agnostic; both
>      overloads are implemented internally by including the min and max
>      values in ALLOCSPI. For the variant where app developers provide a
>      specific SPI (which is also useful in testing), Android simply sets
>      both the min and max parameters to that exact value. Our
>      understanding
>      of #xfrm_alloc_spi  is that min/max are required for ALLOCSPI, and
>      otherwise ENOENT will be returned.
>      Note that we also use the DADDR as a mandatory part of the tuple
>      because of the issue mentioned above: SPIs are only unique in
>      conjunction with a DADDR, regardless of direction, and accordingly,
>      that’s how Android is expecting the uniqueness requirement be
>      enforced. In this way, 5 duplicate SPIs can be used on 5 unique IP
>      addresses on the same machine; therefore, a strict "SPI only"
>      interpretation for ALLOCSPI (or SPI handling in general) is curious.
>      We feel that ALLOCSPI should really enforce the same uniqueness
>      requirements as the SAD.
>      Best,
>      Nathan and Yan
>      -Nathan
>      On Wed, Feb 18, 2026 at 12:42 AM Tobias Brunner
>      <[4]tobias@strongswan.org> wrote:
>      >
>      > Hi Yan,
>      >
>      > > For every inbound SA, we allocate SPIs before negotiation. For
>      > > outbound SAs, we allocate SPIs once requested by the peer. We
>      only
>      > > require the (SPI, destination address) combo to be unique. Thus,
>      we
>      > > may have an inbound and outbound SA sharing an SPI with
>      different
>      > > destinations, or multiple outbound SAs to different peers
>      sharing an
>      > > SPI.
>      >
>      > That should still be allowed when using the intended APIs (i.e.
>      ALLOCSPI
>      > for the inbound and NEWSA for the outbound SA).  ALLOCSPI might
>      enforce
>      > a unique SPI without considering the address, as that's intended
>      for
>      > local, inbound SAs, where the kernel has full control (looking at
>      the
>      > the patch, it's certainly not ideal, as it goes through all
>      installed
>      > SAs to find a duplicate and it prevents an inbound SPI that
>      matches an
>      > existing outbound SPI - I guess that could be resolved by using
>      separate
>      > tables for in- and outbound SAs).  But that must not prevent
>      installing
>      > outbound SAs with the same SPI to another peer using NEWSA, which
>      still
>      > uses a hash that includes the destination address (that must
>      always be
>      > the case because peers are free to allocate whatever SPI they
>      want).
>      >
>      > >> If so, why would you use ALLOCSPI and not just install the
>      outbound SA? Is it to avoid differences for in- and outbound SAs
>      (ALLOCSPI+UPDSA vs. NEWSA)?"
>      > >
>      > > Exactly—it is primarily for code symmetry. By using ALLOCSPI +
>      UPDSA
>      > > for both directions, we maintain a consistent larval lifecycle
>      and
>      > > make it easier to maintain.
>      >
>      > In my opinion, you are using the API incorrectly.  ALLOCSPI is
>      intended
>      > to allocate a free local SPI for an inbound SA.  That is, reserve
>      it
>      > before and while the details of the SA are negotiated with the
>      peer
>      > using IKE.  This step isn't necessary for outbound SAs and forcing
>      such
>      > an allocation, after all the details are known, to the responder's
>      SPI
>      > (which I assume you do via min/max option) doesn't feel right.  I
>      also
>      > don't think there are any benefits in that "consistent larval
>      lifecycle"
>      > (if you found any, please let us know).  And the difference
>      between
>      > UPDSA and NEWSA is the nlmsg_type (there are some attributes that
>      are
>      > different for in- and outbound SAs, especially if you set the
>      direction
>      > in newer kernels, but that's the case regardless of the message
>      type).
>      >
>      > By the way, are you using the min/max option for inbound SAs as
>      well,
>      > with an SPI generated in userland?  That would seem like a
>      violation of
>      > the intention of the API as well (i.e. letting the kernel control
>      the
>      > local SPIs).
>      >
>      > As the XFRM API basically mirrors PF_KEYv2 here, you can find more
>      about
>      > the two ways to install SAs in RFC 2367 (SADB_GETSPI/UPDATE vs.
>      SADB_ADD).
>      >
>      > Regards,
>      > Tobias
>      >
> 
>    --
> 
>    --
>    Best,
>    Yan
> 
> References
> 
>    1. mailto:nharold@google.com
>    2. https://developer.android.com/reference/android/net/IpSecManager#allocateSecurityParameterIndex(java.net.InetAddress)
>    3. https://developer.android.com/reference/android/net/IpSecManager#allocateSecurityParameterIndex(java.net.InetAddress, int)
>    4. mailto:tobias@strongswan.org

[-- Attachment #2: 0001-xfrm-allow-same-SPI-value-for-inbound-and-outbound-S.patch --]
[-- Type: text/x-diff, Size: 3548 bytes --]

From b712c3ff7d586c8ed62005e6a37ae01c4a771ce8 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony.antony@secunet.com>
Date: Wed, 25 Mar 2026 06:02:07 +0100
Subject: [PATCH] xfrm: allow same SPI value for inbound and outbound SAs

Commit 94f39804d891 ("xfrm: Duplicate SPI Handling") introduced
xfrm_state_lookup_spi_proto() to fix duplicate SPI allocation for
inbound SAs with different destination addresses.  It enforces global
uniqueness by (spi, proto) across all states regardless of direction.

When x->dir is set, use xfrm_state_lookup_spi_proto() with a strict
per-direction match so that IN and OUT SAs can share a SPI value.

When x->dir is not set, also legacy states created via PF_KEY or without
direction, restore pre-94f39804d891 behavior and check uniqueness by
(daddr, spi, proto) via xfrm_state_lookup(). Extract the mark from
x->mark as required by xfrm_state_lookup().

Move the x->dir assignment in xfrm_alloc_userspi() to before
xfrm_alloc_spi() lookup using dir when it is set.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c | 14 ++++++++++++--
 net/xfrm/xfrm_user.c  |  6 +++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 98b362d51836..1cc4a06f9163 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1698,7 +1698,9 @@ 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)
+static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net,
+						      __be32 spi, u8 proto,
+						      u8 dir)
 {
 	struct xfrm_state *x;
 	unsigned int i;
@@ -1707,6 +1709,8 @@ static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net, __be32 sp
 	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 (x->dir != dir)
+					continue;
 				if (!xfrm_state_hold_rcu(x))
 					continue;
 				rcu_read_unlock();
@@ -2577,6 +2581,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
 	struct xfrm_state *x0;
 	int err = -ENOENT;
 	u32 range = high - low + 1;
+	u32 mark = x->mark.v & x->mark.m;
 	__be32 newspi = 0;
 
 	spin_lock_bh(&x->lock);
@@ -2598,7 +2603,12 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
 		newspi = htonl(spi);
 
 		spin_lock_bh(&net->xfrm.xfrm_state_lock);
-		x0 = xfrm_state_lookup_spi_proto(net, newspi, x->id.proto);
+		if (x->dir)
+			x0 = xfrm_state_lookup_spi_proto(net, newspi,
+							 x->id.proto, x->dir);
+		else
+			x0 = xfrm_state_lookup(net, mark, &x->id.daddr, newspi,
+					       x->id.proto, x->props.family);
 		if (!x0) {
 			x->id.spi = newspi;
 			h = xfrm_spi_hash(net, &x->id.daddr, newspi, x->id.proto, x->props.family);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 403b5ecac2c5..98b90c747aad 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1873,13 +1873,13 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out_noput;
 	}
 
+	if (attrs[XFRMA_SA_DIR])
+		x->dir = nla_get_u8(attrs[XFRMA_SA_DIR]);
+
 	err = xfrm_alloc_spi(x, p->min, p->max, extack);
 	if (err)
 		goto out;
 
-	if (attrs[XFRMA_SA_DIR])
-		x->dir = nla_get_u8(attrs[XFRMA_SA_DIR]);
-
 	resp_skb = xfrm_state_netlink(skb, x, nlh->nlmsg_seq);
 	if (IS_ERR(resp_skb)) {
 		err = PTR_ERR(resp_skb);
-- 
2.39.5

Re: [EXTERNAL] Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI Handling"

[Antony Antony]

[-- Attachment #1: Type: text/plain, Size: 13150 bytes --]

Hi,
I have tweaked the patch a bit more, uniqueness is only when
x->dir == XFRM_SA_DIR_IN. See the attached patch.

I have added tags Fixes: 
and 
Reported-by: Yan Yan <evitayan@google.com>  
Yan, are you ok with this?

So fart I don't have any tests for this, so more tags welcome:)

regards,
-antony

PS: recent libreswan is setting direction. Thta should not be problem.

On Mon, Mar 30, 2026 at 20:34:07 +0000, Aakash Kumar Shankarappa wrote:
>    Hi Antony,
> 
>    Thanks for the patch. Yes the x->dir based gating approach looks good
>    to me and it works for Marvell.
> 
>    Also this seems like the right direction. It preserves backward
>    compatibility for existing users, while still allowing strict RFC 4301
>    complaint SPI uniqueness as an opt-in feature. Anybody who wants the
>    stricter behaviour can upgrade to Strongswan 6.0.0+ and leverage
>    XFRM_SA_DIR_IN.
> 
>    Thanks, Aakash
> 
>    From: Antony Antony <antony.antony@secunet.com>
>    Date: Monday, 30 March 2026 at 10:24 PM
>    To: Yan Yan <evitayan@google.com>
>    Cc: Nathan Harold <nharold@google.com>, Tobias Brunner
>    <tobias@strongswan.org>, antony.antony@secunet.com
>    <antony.antony@secunet.com>, Steffen Klassert
>    <steffen.klassert@secunet.com>, paul@nohats.ca <paul@nohats.ca>,
>    netdev@vger.kernel.org <netdev@vger.kernel.org>, Herbert Xu
>    <herbert@gondor.apana.org.au>, David S . Miller <davem@davemloft.net>,
>    Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
>    pabeni@redhat.com <pabeni@redhat.com>, horms@kernel.org
>    <horms@kernel.org>, Aakash Kumar Shankarappa
>    <saakashkumar@marvell.com>, akamluddin@marvell.com
>    <akamluddin@marvell.com>, greg@kroah.com <greg@kroah.com>
>    Subject: [EXTERNAL] Re: [REGRESSION] Discussion on "xfrm: Duplicate SPI
>    Handling"
>    Prioritize security for external emails:
>    Confirm sender and content safety before clicking links or opening
>    attachments
>    [1]Report Suspicious
> 
> 
>    Hi, I looked into this. I feel a simple solution is use x->dir as
>    Nathan proposed. When dir is not set we get pre commit 94f39804d891
>    ("xfrm: Duplicate SPI Handling") behaviour. When XFRM_SA_DIR is set
>    alloc_spi() returns per direction unique spi. Another benfit is, this
>    would also keep PF_KEY use case as it was before that comit. Here is
>    simple RFC patch attached. How does this look? strongswan 6.0.0, from
>    Dec 2024, sets x->dir. Aakash would this work for for marvell? regads,
>    -antony On Fri, Mar 27, 2026 at 17:05:13 -0700, Yan Yan wrote: > Hi
>    all, > I wanted to send a friendly ping to see if we are aligning on
>    making > the strict global SPI uniqueness requirement optional, perhaps
>    via a > toggle or by leveraging the XFRM_SA_DIR attribute as previously
>    > discussed. > Are there any other questions or concerns regarding this
>    approach, or > anything else we should clarify to ensure backward
>    compatibility while > meeting the needs of modern standards? > Best, >
>    Yan > > On Tue, Feb 24, 2026 at 3:53 PM Nathan Harold
>    <[1]nharold@google.com> > wrote: > > > That should still be allowed
>    when using the intended APIs (i.e. > ALLOCSPI > > for the inbound and
>    NEWSA for the outbound SA). ALLOCSPI might > enforce > > a unique SPI
>    without considering the address, as that's intended > for > > local,
>    inbound SAs, where the kernel has full control (looking at > the > >
>    the patch, it's certainly not ideal, as it goes through all > installed
>    > > SAs to find a duplicate and it prevents an inbound SPI that >
>    matches an > > existing outbound SPI - I guess that could be resolved
>    by using > separate > > tables for in- and outbound SAs). But that must
>    not prevent > installing > > outbound SAs with the same SPI to another
>    peer using NEWSA, which > still > > uses a hash that includes the
>    destination address (that must > always be > > the case because peers
>    are free to allocate whatever SPI they > want). > Agreed that there are
>    some unfortunate limitations with the current > patch. Keying off the
>    inclusion of XFRM_SA_DIR would resolve the > issue > you noted
>    (conflating inbound and outbound SPIs) and function as an > opt-in for
>    this enforcement. Whatever the mechanism though, the new > behavior
>    should be opt-in rather than opt-out in order to maintain > backwards
>    compatibility. > > In my opinion, you are using the API incorrectly...
>    I also > > don't think there are any benefits in that "consistent
>    larval > lifecycle" > > (if you found any, please let us know). > The
>    Android architecture is multi-tenant and allows userspace apps > to >
>    establish SAs. At the time we designed it, this felt like the >
>    cleanest > way to facilitate leak-free resource management because the
>    chain of > associations between kernel resources could be symmetrical
>    (and > managing them was already quite complicated). Mea culpa
>    (Nathan). > But, > correctly or not, it has/had worked for many years.
>    > > By the way, are you using the min/max option for inbound SAs as >
>    well, > > with an SPI generated in userland? That would seem like a >
>    violation of > > the intention of the API as well (i.e. letting the
>    kernel control > the > > local SPIs). > We provide following two
>    Android APIs for app developers: >
>    [2][2]https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.an
>    droid.com_reference_android_net_IpSecManager-23&d=DwIDaQ&c=nKjWec2b6R0m
>    OyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWI
>    SL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=9txNFXC-wFiRFV_Q
>    JlIQLW2AWSbERIOWcGHJfuQP2ZA&e= >
>    allocateSecurityParameterIndex(java.net.InetAddress) >
>    [3][3]https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.an
>    droid.com_reference_android_net_IpSecManager-23&d=DwIDaQ&c=nKjWec2b6R0m
>    OyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWI
>    SL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=9txNFXC-wFiRFV_Q
>    JlIQLW2AWSbERIOWcGHJfuQP2ZA&e= >
>    allocateSecurityParameterIndex(java.net.InetAddress,%20int) > Indeed,
>    allocateSecurityParameterIndex is direction-agnostic; both > overloads
>    are implemented internally by including the min and max > values in
>    ALLOCSPI. For the variant where app developers provide a > specific SPI
>    (which is also useful in testing), Android simply sets > both the min
>    and max parameters to that exact value. Our > understanding > of
>    #xfrm_alloc_spi is that min/max are required for ALLOCSPI, and >
>    otherwise ENOENT will be returned. > Note that we also use the DADDR as
>    a mandatory part of the tuple > because of the issue mentioned above:
>    SPIs are only unique in > conjunction with a DADDR, regardless of
>    direction, and accordingly, > that’s how Android is expecting the
>    uniqueness requirement be > enforced. In this way, 5 duplicate SPIs can
>    be used on 5 unique IP > addresses on the same machine; therefore, a
>    strict "SPI only" > interpretation for ALLOCSPI (or SPI handling in
>    general) is curious. > We feel that ALLOCSPI should really enforce the
>    same uniqueness > requirements as the SAD. > Best, > Nathan and Yan >
>    -Nathan > On Wed, Feb 18, 2026 at 12:42 AM Tobias Brunner >
>    <[4]tobias@strongswan.org> wrote: > > > > Hi Yan, > > > > > For every
>    inbound SA, we allocate SPIs before negotiation. For > > > outbound
>    SAs, we allocate SPIs once requested by the peer. We > only > > >
>    require the (SPI, destination address) combo to be unique. Thus, > we >
>    > > may have an inbound and outbound SA sharing an SPI with > different
>    > > > destinations, or multiple outbound SAs to different peers >
>    sharing an > > > SPI. > > > > That should still be allowed when using
>    the intended APIs (i.e. > ALLOCSPI > > for the inbound and NEWSA for
>    the outbound SA). ALLOCSPI might > enforce > > a unique SPI without
>    considering the address, as that's intended > for > > local, inbound
>    SAs, where the kernel has full control (looking at > the > > the patch,
>    it's certainly not ideal, as it goes through all > installed > > SAs to
>    find a duplicate and it prevents an inbound SPI that > matches an > >
>    existing outbound SPI - I guess that could be resolved by using >
>    separate > > tables for in- and outbound SAs). But that must not
>    prevent > installing > > outbound SAs with the same SPI to another peer
>    using NEWSA, which > still > > uses a hash that includes the
>    destination address (that must > always be > > the case because peers
>    are free to allocate whatever SPI they > want). > > > > >> If so, why
>    would you use ALLOCSPI and not just install the > outbound SA? Is it to
>    avoid differences for in- and outbound SAs > (ALLOCSPI+UPDSA vs.
>    NEWSA)?" > > > > > > Exactly—it is primarily for code symmetry. By
>    using ALLOCSPI + > UPDSA > > > for both directions, we maintain a
>    consistent larval lifecycle > and > > > make it easier to maintain. > >
>    > > In my opinion, you are using the API incorrectly. ALLOCSPI is >
>    intended > > to allocate a free local SPI for an inbound SA. That is,
>    reserve > it > > before and while the details of the SA are negotiated
>    with the > peer > > using IKE. This step isn't necessary for outbound
>    SAs and forcing > such > > an allocation, after all the details are
>    known, to the responder's > SPI > > (which I assume you do via min/max
>    option) doesn't feel right. I > also > > don't think there are any
>    benefits in that "consistent larval > lifecycle" > > (if you found any,
>    please let us know). And the difference > between > > UPDSA and NEWSA
>    is the nlmsg_type (there are some attributes that > are > > different
>    for in- and outbound SAs, especially if you set the > direction > > in
>    newer kernels, but that's the case regardless of the message > type). >
>    > > > By the way, are you using the min/max option for inbound SAs as >
>    well, > > with an SPI generated in userland? That would seem like a >
>    violation of > > the intention of the API as well (i.e. letting the
>    kernel control > the > > local SPIs). > > > > As the XFRM API basically
>    mirrors PF_KEYv2 here, you can find more > about > > the two ways to
>    install SAs in RFC 2367 (SADB_GETSPI/UPDATE vs. > SADB_ADD). > > > >
>    Regards, > > Tobias > > > > -- > > -- > Best, > Yan > > References > >
>    1. mailto:nharold@google.com > 2.
>    [4]https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.andro
>    id.com_reference_android_net_IpSecManager-23allocateSecurityParameterIn
>    dex-28java.net.InetAddress-29&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn
>    5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51r
>    d5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=UtpwkuoswT-6aK0he6dSS-0dvZfIcRjbfj
>    _Eu4A-c6E&e= > 3.
>    [5]https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.andro
>    id.com_reference_android_net_IpSecManager-23allocateSecurityParameterIn
>    dex-28java.net.InetAddress&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn5Ln
>    Vsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51rd5M
>    6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=fVmYoLIpyuMX3AtAkU7ejR1dno_8QtnhCKLMD
>    4BmURc&e=, int) > 4. mailto:tobias@strongswan.org
> 
> References
> 
>    Visible links:
>    1. https://us-phishalarm-ewt.proofpoint.com/EWT/v1/CRVmXkqW!tG3Tv5d8inv1_6DXc1X1B4ctthRq2qCkR8nIF_n_SOJiXQ-SqG_LUk--J5LZEwV9jGdXABQriDruLYnPEg$
>    2. https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.android.com_reference_android_net_IpSecManager-23&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=9txNFXC-wFiRFV_QJlIQLW2AWSbERIOWcGHJfuQP2ZA&e=
>    3. https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.android.com_reference_android_net_IpSecManager-23&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=9txNFXC-wFiRFV_QJlIQLW2AWSbERIOWcGHJfuQP2ZA&e=
>    4. https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.android.com_reference_android_net_IpSecManager-23allocateSecurityParameterIndex-28java.net.InetAddress-29&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=UtpwkuoswT-6aK0he6dSS-0dvZfIcRjbfj_Eu4A-c6E&e=
>    5. https://urldefense.proofpoint.com/v2/url?u=https-3A__developer.android.com_reference_android_net_IpSecManager-23allocateSecurityParameterIndex-28java.net.InetAddress&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=r6Wzn5LnVsk7Tgc5x4l_c04I_Hr_8TYqFn-YFi_gjqI&m=JsnNMKKypJWISL5BbGkfA2yD1_H51rd5M6YO4NG3WpRdteRwP5OdfTJFNEK0Xiec&s=fVmYoLIpyuMX3AtAkU7ejR1dno_8QtnhCKLMD4BmURc&e=
> 
>    Hidden links:
>    7. https://aka.ms/GetOutlookForMac

[-- Attachment #2: 0001-xfrm-enforce-SPI-uniqueness-for-inbound-SAs-only.patch --]
[-- Type: text/x-diff, Size: 3671 bytes --]

From 47de0cbe52f1a872edfbeb9c2ad5f08c564fad43 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony.antony@secunet.com>
Date: Wed, 25 Mar 2026 06:02:07 +0100
Subject: [PATCH] xfrm: enforce SPI uniqueness for inbound SAs only

Per RFC 4301 section 4.4.2.1, the SPI is selected by the receiving
end, which is interpreted as making SPI uniqueness an inbound-only
requirement.

Commit 94f39804d891 ("xfrm: Duplicate SPI Handling") introduced
xfrm_state_lookup_spi_proto() to fix duplicate SPI allocation for
inbound SAs with different destination addresses.  However, it enforces
global uniqueness by (spi, proto) across all states regardless of
direction, which causes SPI allocation to fail for outbound SAs when
the same (spi, proto) is already in use by an inbound SA.

When x->dir == XFRM_DIR_IN, enforce SPI uniqueness via
xfrm_state_lookup_spi_proto() scoped to inbound SAs.  SAs created via
PF_KEY, without direction, or with XFRM_DIR_OUT restore the
pre-94f39804d891 RFC 2401 lookup by (daddr, spi, proto).

Reported-by: Yan Yan <evitayan@google.com>
Fixes: 94f39804d891 ("xfrm: Duplicate SPI Handling")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c | 14 ++++++++++++--
 net/xfrm/xfrm_user.c  |  6 +++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 98b362d51836..66cff3e8dd65 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1698,7 +1698,9 @@ 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)
+static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net,
+						      __be32 spi, u8 proto,
+						      u8 dir)
 {
 	struct xfrm_state *x;
 	unsigned int i;
@@ -1707,6 +1709,8 @@ static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net, __be32 sp
 	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 (x->dir != dir)
+					continue;
 				if (!xfrm_state_hold_rcu(x))
 					continue;
 				rcu_read_unlock();
@@ -2577,6 +2581,7 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
 	struct xfrm_state *x0;
 	int err = -ENOENT;
 	u32 range = high - low + 1;
+	u32 mark = x->mark.v & x->mark.m;
 	__be32 newspi = 0;
 
 	spin_lock_bh(&x->lock);
@@ -2598,7 +2603,12 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
 		newspi = htonl(spi);
 
 		spin_lock_bh(&net->xfrm.xfrm_state_lock);
-		x0 = xfrm_state_lookup_spi_proto(net, newspi, x->id.proto);
+		if (x->dir == XFRM_SA_DIR_IN)
+			x0 = xfrm_state_lookup_spi_proto(net, newspi,
+							 x->id.proto, x->dir);
+		else
+			x0 = xfrm_state_lookup(net, mark, &x->id.daddr, newspi,
+					       x->id.proto, x->props.family);
 		if (!x0) {
 			x->id.spi = newspi;
 			h = xfrm_spi_hash(net, &x->id.daddr, newspi, x->id.proto, x->props.family);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 403b5ecac2c5..98b90c747aad 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1873,13 +1873,13 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out_noput;
 	}
 
+	if (attrs[XFRMA_SA_DIR])
+		x->dir = nla_get_u8(attrs[XFRMA_SA_DIR]);
+
 	err = xfrm_alloc_spi(x, p->min, p->max, extack);
 	if (err)
 		goto out;
 
-	if (attrs[XFRMA_SA_DIR])
-		x->dir = nla_get_u8(attrs[XFRMA_SA_DIR]);
-
 	resp_skb = xfrm_state_netlink(skb, x, nlh->nlmsg_seq);
 	if (IS_ERR(resp_skb)) {
 		err = PTR_ERR(resp_skb);
-- 
2.39.5