Netdev List
 help / color / mirror / Atom feed
* [PATCH net] calipso: update payload_len when removing the CALIPSO option
@ 2026-10-07 15:25 Joas Antonio dos Santos
  2026-10-07 20:45 ` Eric Dumazet
  0 siblings, 1 reply; 2+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-07 15:25 UTC (permalink / raw)
  To: Paul Moore
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-security-module

calipso_skbuff_delattr() removes the CALIPSO option from the hop-by-hop
extension header, or the whole extension header if CALIPSO is its only
option, and pulls the skb by the removed length.  It never updates
ipv6hdr->payload_len, so after a successful removal the header still
claims the original length.

calipso_skbuff_setattr() already adjusts payload_len by the length it
adds; the delete path was missed.

This is reached on the forward path when a packet carrying CALIPSO is
sent to a destination mapped to an unlabeled NetLabel domain.  The
forwarded packet then has a payload_len larger than its real payload
by the removed length (8 to 264 bytes), and the receiver drops it as
truncated.

Adjust payload_len after the header is moved.

Fixes: 2917f57b6bc1 ("calipso: Allow the lsm to label the skbuff directly.")
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
Found by code review.  Compile-tested only (arm64, W=1); not runtime-tested.

 net/ipv6/calipso.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index c6a34334e..c072eca50 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -1428,6 +1428,9 @@ static int calipso_skbuff_delattr(struct sk_buff *skb)
 		skb_pull(skb, delta);
 		memmove((char *)ip6_hdr + delta, ip6_hdr, size);
 		skb_reset_network_header(skb);
+		ip6_hdr = ipv6_hdr(skb);
+		ip6_hdr->payload_len =
+			htons(ntohs(ip6_hdr->payload_len) - delta);
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] calipso: update payload_len when removing the CALIPSO option
  2026-10-07 15:25 [PATCH net] calipso: update payload_len when removing the CALIPSO option Joas Antonio dos Santos
@ 2026-10-07 20:45 ` Eric Dumazet
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Dumazet @ 2026-10-07 20:45 UTC (permalink / raw)
  To: Joas Antonio dos Santos
  Cc: Paul Moore, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-security-module, Alice Mikityanska

Le mer. 7 oct. 2026 à 17:26, Joas Antonio dos Santos
<joasantonio108@gmail.com> a écrit :
>
> calipso_skbuff_delattr() removes the CALIPSO option from the hop-by-hop
> extension header, or the whole extension header if CALIPSO is its only
> option, and pulls the skb by the removed length.  It never updates
> ipv6hdr->payload_len, so after a successful removal the header still
> claims the original length.
>
> calipso_skbuff_setattr() already adjusts payload_len by the length it
> adds; the delete path was missed.
>
> This is reached on the forward path when a packet carrying CALIPSO is
> sent to a destination mapped to an unlabeled NetLabel domain.  The
> forwarded packet then has a payload_len larger than its real payload
> by the removed length (8 to 264 bytes), and the receiver drops it as
> truncated.
>
> Adjust payload_len after the header is moved.
>
> Fixes: 2917f57b6bc1 ("calipso: Allow the lsm to label the skbuff directly.")
> Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
> Assisted-by: Claude:claude-opus-5-5
> ---
> Found by code review.  Compile-tested only (arm64, W=1); not runtime-tested.

The analysis looks right to me, but this means removing the label on
the forward path has been broken since 2016 for every packet that is
not resegmented later (SYN, UDP, ICMPv6...).

For a patch targeting net and stable, we would really like to see it
exercised at least once. Please run a forwarding setup with an
unlabeled destination (the NetLabel tests in selinux-testsuite could
be a starting point) and describe what you tested in the changelog.

>
>  net/ipv6/calipso.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
> index c6a34334e..c072eca50 100644
> --- a/net/ipv6/calipso.c
> +++ b/net/ipv6/calipso.c
> @@ -1428,6 +1428,9 @@ static int calipso_skbuff_delattr(struct sk_buff *skb)
>                 skb_pull(skb, delta);
>                 memmove((char *)ip6_hdr + delta, ip6_hdr, size);
>                 skb_reset_network_header(skb);
> +               ip6_hdr = ipv6_hdr(skb);
> +               ip6_hdr->payload_len =
> +                       htons(ntohs(ip6_hdr->payload_len) - delta);

This is not correct for BIG TCP.

GRO can aggregate TCP packets carrying identical extension headers,
and ipv6_gro_complete() uses ipv6_set_payload_len(), which stores 0
when the packet is bigger than IPV6_MAXPLEN.

For such a forwarded packet, the above would write
htons(65536 - delta), and ipv6_payload_len() would no longer fall
back to skb->len. nf_tables, conntrack and sch_cake rely on it.

Something like this instead:

ip6_hdr = ipv6_hdr(skb);
/* BIG TCP packets have payload_len == 0 */
if (ip6_hdr->payload_len)
be16_add_cpu(&ip6_hdr->payload_len, -delta);

calipso_skbuff_setattr() has the same issue with its
htons(payload + len_delta), this can be addressed in a separate patch.

Thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-07 20:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07 15:25 [PATCH net] calipso: update payload_len when removing the CALIPSO option Joas Antonio dos Santos
2026-10-07 20:45 ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox