[PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[Jérémy Jean]

A reassembled skb retains the secpath from its first fragment, which
makes it possible for the final policy check to accept bytes that were
not covered by the transform recorded on the skb. This can affect IPsec
tunnel traffic, including AH and ESP, and allow a packet to be accepted
even though only part of its payload was authenticated.

Reject a fragment when its secpath differs from the fragments already
in the queue, and keep overlap statistics limited to actual overlaps.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 include/net/inet_frag.h  |  1 +
 include/net/xfrm.h       | 17 +++++++++++++++++
 net/ipv4/inet_fragment.c |  5 +++++
 net/ipv4/ip_fragment.c   |  3 ++-
 net/ipv6/reassembly.c    |  3 ++-
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index 365925c9d2628..b02d2472b003e 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -171,6 +171,7 @@ extern const u8 ip_frag_ecn_table[16];
 #define IPFRAG_OK	0
 #define IPFRAG_DUP	1
 #define IPFRAG_OVERLAP	2
+#define IPFRAG_MIXED_SECPATH	3
 int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
 			   int offset, int end);
 void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a6d69aaa6cd2d..493faac0faa45 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1201,6 +1201,23 @@ secpath_reset(struct sk_buff *skb)
 #endif
 }
 
+static inline bool
+secpath_equal(const struct sk_buff *skb1, const struct sk_buff *skb2)
+{
+	const struct sec_path *sp1 = skb_sec_path(skb1);
+	const struct sec_path *sp2 = skb_sec_path(skb2);
+	int i;
+
+	if (!sp1 || !sp2)
+		return sp1 == sp2;
+	if (sp1->len != sp2->len || sp1->verified_cnt != sp2->verified_cnt)
+		return false;
+	for (i = 0; i < sp1->len; i++)
+		if (sp1->xvec[i] != sp2->xvec[i])
+			return false;
+	return true;
+}
+
 static inline int
 xfrm_addr_any(const xfrm_address_t *addr, unsigned short family)
 {
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index b286ee429da8f..c18da91c1f031 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -23,6 +23,7 @@
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
+#include <net/xfrm.h>
 
 /* Use skb->cb to track consecutive/adjacent fragments coming at
  * the end of the queue. Nodes in the rb-tree queue will
@@ -441,6 +442,10 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
 {
 	struct sk_buff *last = q->fragments_tail;
 
+	/* The reassembled skb keeps the first fragment's secpath. */
+	if (last && !secpath_equal(last, skb))
+		return IPFRAG_MIXED_SECPATH;
+
 	/* An IP fragment is never a GSO packet, but an untrusted source
 	 * (virtio_net_hdr) may have attached GSO metadata to it. Do not let
 	 * that reach the reassembled skb, whose head keeps the first
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index c790d2f494870..36105dd192823 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -378,8 +378,9 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb, int *refs)
 		err = -EINVAL;
 		goto err;
 	}
+	if (err == IPFRAG_OVERLAP)
+		__IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 	err = -EINVAL;
-	__IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 discard_qp:
 	inet_frag_kill(&qp->q, refs);
 	__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 05c51f669754a..53a685f5c6b7f 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -240,8 +240,9 @@ static int ip6_frag_queue(struct net *net,
 		err = -EINVAL;
 		goto err;
 	}
+	if (err == IPFRAG_OVERLAP)
+		__IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
 	err = -EINVAL;
-	__IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
 discard_fq:
 	inet_frag_kill(&fq->q, refs);
 	__IP6_INC_STATS(net, idev, IPSTATS_MIB_REASMFAILS);
-- 
2.47.3

Re: [PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[Eric Dumazet]

Le ven. 23 oct. 2026 à 09:45, Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> a écrit :
>
> A reassembled skb retains the secpath from its first fragment, which
> makes it possible for the final policy check to accept bytes that were
> not covered by the transform recorded on the skb. This can affect IPsec
> tunnel traffic, including AH and ESP, and allow a packet to be accepted
> even though only part of its payload was authenticated.
>
> Reject a fragment when its secpath differs from the fragments already
> in the queue, and keep overlap statistics limited to actual overlaps.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  include/net/inet_frag.h  |  1 +
>  include/net/xfrm.h       | 17 +++++++++++++++++
>  net/ipv4/inet_fragment.c |  5 +++++
>  net/ipv4/ip_fragment.c   |  3 ++-
>  net/ipv6/reassembly.c    |  3 ++-
>  5 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index 365925c9d2628..b02d2472b003e 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -171,6 +171,7 @@ extern const u8 ip_frag_ecn_table[16];
>  #define IPFRAG_OK      0
>  #define IPFRAG_DUP     1
>  #define IPFRAG_OVERLAP 2
> +#define IPFRAG_MIXED_SECPATH   3
>  int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>                            int offset, int end);
>  void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index a6d69aaa6cd2d..493faac0faa45 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1201,6 +1201,23 @@ secpath_reset(struct sk_buff *skb)
>  #endif
>  }
>
> +static inline bool
> +secpath_equal(const struct sk_buff *skb1, const struct sk_buff *skb2)
> +{
> +       const struct sec_path *sp1 = skb_sec_path(skb1);
> +       const struct sec_path *sp2 = skb_sec_path(skb2);
> +       int i;
> +
> +       if (!sp1 || !sp2)
> +               return sp1 == sp2;
> +       if (sp1->len != sp2->len || sp1->verified_cnt != sp2->verified_cnt)
> +               return false;

verified_cnt is policy check bookkeeping (written by
__xfrm_policy_check()), it says nothing about which states
authenticated the payload. I would only compare len and xvec[].

With the verified_cnt part removed:

Reviewed-by: Eric Dumazet <edumazet@kernel.org>



> +       for (i = 0; i < sp1->len; i++)
> +               if (sp1->xvec[i] != sp2->xvec[i])
> +                       return false;
> +       return true;
> +}
> +
>  static inline int
>  xfrm_addr_any(const xfrm_address_t *addr, unsigned short family)
>  {
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index b286ee429da8f..c18da91c1f031 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -23,6 +23,7 @@
>  #include <net/inet_ecn.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
> +#include <net/xfrm.h>
>
>  /* Use skb->cb to track consecutive/adjacent fragments coming at
>   * the end of the queue. Nodes in the rb-tree queue will
> @@ -441,6 +442,10 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>  {
>         struct sk_buff *last = q->fragments_tail;
>
> +       /* The reassembled skb keeps the first fragment's secpath. */
> +       if (last && !secpath_equal(last, skb))
> +               return IPFRAG_MIXED_SECPATH;
> +
>         /* An IP fragment is never a GSO packet, but an untrusted source
>          * (virtio_net_hdr) may have attached GSO metadata to it. Do not let
>          * that reach the reassembled skb, whose head keeps the first
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index c790d2f494870..36105dd192823 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -378,8 +378,9 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb, int *refs)
>                 err = -EINVAL;
>                 goto err;
>         }
> +       if (err == IPFRAG_OVERLAP)
> +               __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
>         err = -EINVAL;
> -       __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
>  discard_qp:
>         inet_frag_kill(&qp->q, refs);
>         __IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 05c51f669754a..53a685f5c6b7f 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -240,8 +240,9 @@ static int ip6_frag_queue(struct net *net,
>                 err = -EINVAL;
>                 goto err;
>         }
> +       if (err == IPFRAG_OVERLAP)
> +               __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
>         err = -EINVAL;
> -       __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
>  discard_fq:
>         inet_frag_kill(&fq->q, refs);
>         __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASMFAILS);
> --
> 2.47.3
>

Re: [PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix misses IPsec tunnel devices that strip the secpath (VTI,
  and xfrmi across netns), so partially authenticated reassembly can…
- [Medium] secpath_equal() (include/net/xfrm.h:1213-1218) requires every
  fragment to carry identical xfrm_state pointers and an identical…
- [Medium] secpath_equal() treats sp->verified_cnt as stable fragment
  metadata, but it can change after a fragment is queued.

--

inet: frags: prevent acceptance of partially authenticated reassembly

This patch adds secpath_equal(). It makes inet_frag_queue_insert() reject
a fragment whose secpath differs from the queue tail, so a reassembled
skb cannot carry a secpath that covers only part of its payload. It also
limits the REASM_OVERLAPS counter to actual overlaps.

> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index a6d69aaa6cd2d..493faac0faa45 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1201,6 +1201,23 @@ secpath_reset(struct sk_buff *skb)
>  #endif
>  }
>  
> +static inline bool
> +secpath_equal(const struct sk_buff *skb1, const struct sk_buff *skb2)
> +{
> +	const struct sec_path *sp1 = skb_sec_path(skb1);
> +	const struct sec_path *sp2 = skb_sec_path(skb2);
> +	int i;
> +
> +	if (!sp1 || !sp2)
> +		return sp1 == sp2;
> +	if (sp1->len != sp2->len || sp1->verified_cnt != sp2->verified_cnt)

[Severity: Medium]
Is verified_cnt stable enough to compare here?

Clones share the skb_ext allocation, because __skb_ext_copy() only bumps
ext->refcnt. __xfrm_policy_check() gets sp through skb_sec_path() with no
copy-on-write and writes the shared field in place, without q->lock:

net/xfrm/xfrm_policy.c:__xfrm_policy_check() {
    ...
	sp->verified_cnt = k;
    ...
}

One way to hit this is an IPsec gateway that forwards tunnel-mode
decapsulated inner fragments, with an AF_PACKET fanout socket using
PACKET_FANOUT_FLAG_DEFRAG on the receiving device:

packet_rcv_fanout()
  ip_check_defrag()
    skb_share_check()   <- clone of fragment 1 queued, verified_cnt == 0
    ip_defrag()

The original fragment 1 then continues through:

ip_forward()
  xfrm4_policy_check(NULL, XFRM_POLICY_FWD, skb)
    __xfrm_policy_check()   <- shared sp->verified_cnt = k

Fragment 2 then arrives with verified_cnt == 0. Wouldn't secpath_equal()
return false here, so that ip_frag_queue() kills the IP_DEFRAG_AF_PACKET
queue and the AF_PACKET consumer never gets these datagrams?

tc mirred (tcf_mirred_to_dev()->skb_clone()) looks like another way to
end up with a shared secpath across skbs that are processed independently.

On different CPUs, this also looks like a data race between the unlocked
write in __xfrm_policy_check() and this read under q->lock.

> +		return false;
> +	for (i = 0; i < sp1->len; i++)
> +		if (sp1->xvec[i] != sp2->xvec[i])
> +			return false;

[Severity: Medium]
Can this reject legitimate datagrams whose fragments were protected by
different SAs that each satisfy the policy?

Two cases where xvec[i] could differ between fragments of one datagram:

  - per-CPU SAs (pcpu_num / XFRMA_SA_PCPU) on the sending gateway, where
    the fragments of a forwarded datagram are steered to different CPUs.
    For example, UDP 4-tuple RSS can hash the port-bearing first fragment
    differently from the rest.

  - a datagram whose fragments straddle a make-before-break rekey.

Before this patch these datagrams were reassembled and passed the final
policy check.

The commit message says "Reject a fragment when its secpath differs".
However, IPFRAG_MIXED_SECPATH falls through to the same path as an
overlap in ip_frag_queue():

	err = -EINVAL;
discard_qp:
	inet_frag_kill(&qp->q, refs);
	__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);

It does the same through discard_fq in ip6_frag_queue() and insert_error
in nf_ct_frag6_queue(). So the whole queue is thrown away, not just one
fragment.

Should the commit message mention the whole-queue discard, and the
tightening of matching to SA identity?

The drop is also counted only as IPSTATS_MIB_REASMFAILS, with the default
drop reason. Would a dedicated counter or drop reason make this easier to
diagnose?

> +	return true;
> +}
> +

[ ... ]

> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index b286ee429da8f..c18da91c1f031 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c

[ ... ]

> @@ -441,6 +442,10 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>  {
>  	struct sk_buff *last = q->fragments_tail;
>  
> +	/* The reassembled skb keeps the first fragment's secpath. */
> +	if (last && !secpath_equal(last, skb))
> +		return IPFRAG_MIXED_SECPATH;

[Severity: High]
Does this cover IPsec tunnel devices that strip the secpath before
reassembly, such as VTI, or xfrmi across netns?

vti_rcv_cb() in net/ipv4/ip_vti.c runs xfrm_policy_check() on each
decapsulated inner fragment and then does:

	skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(skb->dev)));
	skb->dev = dev;

vti6_rcv_cb() in net/ipv6/ip6_vti.c does the same.

skb_scrub_packet() always calls skb_ext_reset(), whatever the value of
xnet, and that drops SKB_EXT_SEC_PATH. Inner fragments authenticated by
the VTI tunnel therefore reach reassembly with a NULL secpath. A spoofed
plaintext fragment that arrives on the underlay interface also has a NULL
secpath.

Without a VRF, ip_defrag() gets the same vif from
l3mdev_master_ifindex_rcu(dev) for both kinds of fragment, so they go
into the same queue. secpath_equal() then returns true for NULL == NULL:

	if (!sp1 || !sp2)
		return sp1 == sp2;

ip_frag_reasm() and ip6_frag_reasm() then set skb->dev to the device of
the last fragment to arrive. If that device is vti0, could
attacker-supplied bytes be treated as tunnel-authenticated by
interface-based checks? Examples are rp_filter when conntrack defrags at
PREROUTING, or INPUT rules using -i vti0 after local-deliver defrag.

The subject and commit message say this prevents "acceptance of
partially authenticated reassembly" for "IPsec tunnel traffic". Should
the commit message say that VTI and xfrmi across netns are not covered?
Alternatively, should fragment provenance be tracked beyond the secpath,
for example by recording VTI/xfrm decapsulation or by comparing the input
device or tunnel?

> +
>  	/* An IP fragment is never a GSO packet, but an untrusted source

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261023074515.2347924-2-Jeremy.Jean%40oss.cyber.gouv.fr