Netdev List
 help / color / mirror / Atom feed
* [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
@ 2026-07-05 12:30 Yizhou Zhao
  2026-07-05 17:35 ` Julian Anastasov
  0 siblings, 1 reply; 5+ messages in thread
From: Yizhou Zhao @ 2026-07-05 12:30 UTC (permalink / raw)
  To: netdev
  Cc: Yizhou Zhao, Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
	Florian Westphal, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, lvs-devel, netfilter-devel, coreteam,
	linux-kernel, Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu,
	stable

set_sctp_state() reads the SCTP chunk header again in order to drive the
IPVS SCTP state table.  For IPv6 it computes the offset with
sizeof(struct ipv6hdr), while the surrounding IPVS code uses iph->len from
ip_vs_fill_iph_skb(), where ipv6_find_hdr() has already skipped extension
headers and found the real transport header.

This makes the state machine read from the wrong offset for IPv6 SCTP
packets that carry extension headers.  For example, an INIT packet with an
8-byte destination options header can be scheduled correctly by
sctp_conn_schedule(), but set_sctp_state() reads the first byte of the SCTP
verification tag as a DATA chunk type.  The connection then moves from NONE
to ESTABLISHED instead of INIT1, gets the longer established timeout, and
updates the active/inactive destination counters incorrectly.  This happens
even though the SCTP handshake has not completed.

Use ip_vs_fill_iph_skb() in set_sctp_state() and base the chunk-header
offset on iph.len, matching sctp_conn_schedule() and the SCTP NAT handlers.
For IPv4 and IPv6 packets without extension headers this preserves the
existing offset.

Fixes: 2906f66a5682 ("ipvs: SCTP Trasport Loadbalancing Support")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: Claude-Code:GLM-5.2
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
 net/netfilter/ipvs/ip_vs_proto_sctp.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 63c78a1f3918..6e0fc23be305 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -375,17 +375,15 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
 		int direction, const struct sk_buff *skb)
 {
 	struct sctp_chunkhdr _sctpch, *sch;
 	unsigned char chunk_type;
+	struct ip_vs_iphdr iph;
 	int event, next_state;
-	int ihl, cofs;
+	int cofs;
 
-#ifdef CONFIG_IP_VS_IPV6
-	ihl = cp->af == AF_INET ? ip_hdrlen(skb) : sizeof(struct ipv6hdr);
-#else
-	ihl = ip_hdrlen(skb);
-#endif
+	if (!ip_vs_fill_iph_skb(cp->af, skb, false, &iph))
+		return;
 
-	cofs = ihl + sizeof(struct sctphdr);
+	cofs = iph.len + sizeof(struct sctphdr);
 	sch = skb_header_pointer(skb, cofs, sizeof(_sctpch), &_sctpch);
 	if (sch == NULL)
 		return;
-- 
2.47.3


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

* Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
  2026-07-05 12:30 [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup Yizhou Zhao
@ 2026-07-05 17:35 ` Julian Anastasov
  2026-07-06  8:22   ` Yizhou Zhao
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Anastasov @ 2026-07-05 17:35 UTC (permalink / raw)
  To: Yizhou Zhao
  Cc: netdev, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
	Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, lvs-devel, netfilter-devel, coreteam, linux-kernel,
	Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu, stable


	Hello,

On Sun, 5 Jul 2026, Yizhou Zhao wrote:

> set_sctp_state() reads the SCTP chunk header again in order to drive the
> IPVS SCTP state table.  For IPv6 it computes the offset with
> sizeof(struct ipv6hdr), while the surrounding IPVS code uses iph->len from
> ip_vs_fill_iph_skb(), where ipv6_find_hdr() has already skipped extension
> headers and found the real transport header.
> 
> This makes the state machine read from the wrong offset for IPv6 SCTP
> packets that carry extension headers.  For example, an INIT packet with an
> 8-byte destination options header can be scheduled correctly by
> sctp_conn_schedule(), but set_sctp_state() reads the first byte of the SCTP
> verification tag as a DATA chunk type.  The connection then moves from NONE
> to ESTABLISHED instead of INIT1, gets the longer established timeout, and
> updates the active/inactive destination counters incorrectly.  This happens
> even though the SCTP handshake has not completed.
> 
> Use ip_vs_fill_iph_skb() in set_sctp_state() and base the chunk-header
> offset on iph.len, matching sctp_conn_schedule() and the SCTP NAT handlers.
> For IPv4 and IPv6 packets without extension headers this preserves the
> existing offset.
> 
> Fixes: 2906f66a5682 ("ipvs: SCTP Trasport Loadbalancing Support")
> Cc: stable@vger.kernel.org
> Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
> Reported-by: Ao Wang <wangao@seu.edu.cn>
> Reported-by: Xuewei Feng <fengxw06@126.com>
> Reported-by: Qi Li <qli01@tsinghua.edu.cn>
> Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
> Assisted-by: Claude-Code:GLM-5.2
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> ---
>  net/netfilter/ipvs/ip_vs_proto_sctp.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index 63c78a1f3918..6e0fc23be305 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -375,17 +375,15 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
>  		int direction, const struct sk_buff *skb)
>  {
>  	struct sctp_chunkhdr _sctpch, *sch;
>  	unsigned char chunk_type;
> +	struct ip_vs_iphdr iph;
>  	int event, next_state;
> -	int ihl, cofs;
> +	int cofs;
>  
> -#ifdef CONFIG_IP_VS_IPV6
> -	ihl = cp->af == AF_INET ? ip_hdrlen(skb) : sizeof(struct ipv6hdr);
> -#else
> -	ihl = ip_hdrlen(skb);
> -#endif
> +	if (!ip_vs_fill_iph_skb(cp->af, skb, false, &iph))
> +		return;

	May be it is better starting from ip_vs_set_state()
to provide new arg 'int iph_len/offset' (set to iph.len), down to
state_transition(), sctp_state_transition() and set_sctp_state().
Same for all protos. It should cost less stack and ipv6_find_hdr()
calls and what matters most, correct iph context in case we
have IP+ICMP+TCP (with just two ports or even with TCP flags)
and are scheduling ICMP, i.e. not IP+TCP as usually.

	But what I see is that ip_vs_in_icmp*() are missing
the ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd) call just
after ip_vs_in_stats() and before ip_vs_icmp_xmit() where
we should provide ciph.len. That is why we don't reach the
set_tcp_state() calls to set correct cp->state and timeout
when scheduling related ICMP. So, this should be fixed too.

> -	cofs = ihl + sizeof(struct sctphdr);
> +	cofs = iph.len + sizeof(struct sctphdr);
>  	sch = skb_header_pointer(skb, cofs, sizeof(_sctpch), &_sctpch);
>  	if (sch == NULL)
>  		return;
> -- 
> 2.47.3

Regards

--
Julian Anastasov <ja@ssi.bg>


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

* Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
  2026-07-05 17:35 ` Julian Anastasov
@ 2026-07-06  8:22   ` Yizhou Zhao
  2026-07-06 14:17     ` Julian Anastasov
  0 siblings, 1 reply; 5+ messages in thread
From: Yizhou Zhao @ 2026-07-06  8:22 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netdev, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
	Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, lvs-devel, netfilter-devel, coreteam, linux-kernel,
	Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu, stable

Hi Julian,

Thanks for your review.

> On Jul 6, 2026, at 01:35, Julian Anastasov <ja@ssi.bg> wrote:
> 
> May be it is better starting from ip_vs_set_state()
> to provide new arg 'int iph_len/offset' (set to iph.len), down to
> state_transition(), sctp_state_transition() and set_sctp_state().
> Same for all protos. It should cost less stack and ipv6_find_hdr()
> calls and what matters most, correct iph context in case we
> have IP+ICMP+TCP (with just two ports or even with TCP flags)
> and are scheduling ICMP, i.e. not IP+TCP as usually.

I agree that the already parsed transport-header offset should be 
passed from ip_vs_set_state() down to the protocol state_transition() 
callbacks, instead of reparsing the skb in set_sctp_state(). We will 
send a v2 that does this for SCTP, TCP and the other IPVS protocols 
in one combined fix.

> But what I see is that ip_vs_in_icmp*() are missing
> the ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd) call just
> after ip_vs_in_stats() and before ip_vs_icmp_xmit() where
> we should provide ciph.len. That is why we don't reach the
> set_tcp_state() calls to set correct cp->state and timeout
> when scheduling related ICMP. So, this should be fixed too.

For the ICMP path, I agree that the missing ip_vs_set_state() call is
worth looking at, but using ICMP errors to drive the upper L4 state 
needs some care, because spoofed ICMP packets can match an 
existing embedded tuple before the endpoint TCP/SCTP stack 
performs its own validation. Maybe this change needs further
discussion?

Thanks,
Yizhou


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

* Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
  2026-07-06  8:22   ` Yizhou Zhao
@ 2026-07-06 14:17     ` Julian Anastasov
  2026-07-07  5:38       ` Yizhou Zhao
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Anastasov @ 2026-07-06 14:17 UTC (permalink / raw)
  To: Yizhou Zhao
  Cc: netdev, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
	Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, lvs-devel, netfilter-devel, coreteam, linux-kernel,
	Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu, stable


	Hello,

On Mon, 6 Jul 2026, Yizhou Zhao wrote:

> > On Jul 6, 2026, at 01:35, Julian Anastasov <ja@ssi.bg> wrote:
> > 
> > May be it is better starting from ip_vs_set_state()
> > to provide new arg 'int iph_len/offset' (set to iph.len), down to
> > state_transition(), sctp_state_transition() and set_sctp_state().
> > Same for all protos. It should cost less stack and ipv6_find_hdr()
> > calls and what matters most, correct iph context in case we
> > have IP+ICMP+TCP (with just two ports or even with TCP flags)
> > and are scheduling ICMP, i.e. not IP+TCP as usually.
> 
> I agree that the already parsed transport-header offset should be 
> passed from ip_vs_set_state() down to the protocol state_transition() 
> callbacks, instead of reparsing the skb in set_sctp_state(). We will 
> send a v2 that does this for SCTP, TCP and the other IPVS protocols 
> in one combined fix.
> 
> > But what I see is that ip_vs_in_icmp*() are missing
> > the ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd) call just
> > after ip_vs_in_stats() and before ip_vs_icmp_xmit() where
> > we should provide ciph.len. That is why we don't reach the
> > set_tcp_state() calls to set correct cp->state and timeout
> > when scheduling related ICMP. So, this should be fixed too.
> 
> For the ICMP path, I agree that the missing ip_vs_set_state() call is
> worth looking at, but using ICMP errors to drive the upper L4 state 
> needs some care, because spoofed ICMP packets can match an 
> existing embedded tuple before the endpoint TCP/SCTP stack 
> performs its own validation. Maybe this change needs further
> discussion?

	May be only for the schedule_icmp case while the other
ICMP replies should not change it:

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 7f93239898ff..05fdcf4ce2c0 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1971,6 +1971,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
 
 	/* do the statistics and put it back */
 	ip_vs_in_stats(cp, skb);
+	if (new_cp)
+		ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd, offset);
 	if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol ||
 	    IPPROTO_SCTP == cih->protocol)
 		offset += 2 * sizeof(__u16);

	Here is why schedule_icmp was added:

https://archive.linuxvirtualserver.org/html/lvs-devel/2015-08/msg00015.html

	But the inner TCP header should be generated by the
real server, not from the client, so things can go wrong. We can
leave it as it is now - we will forward the ICMP to the right real 
server by using short conn timeout...

Regards

--
Julian Anastasov <ja@ssi.bg>


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

* Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
  2026-07-06 14:17     ` Julian Anastasov
@ 2026-07-07  5:38       ` Yizhou Zhao
  0 siblings, 0 replies; 5+ messages in thread
From: Yizhou Zhao @ 2026-07-07  5:38 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netdev, Simon Horman, Pablo Neira Ayuso, Florian Westphal,
	Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, lvs-devel, netfilter-devel, coreteam, linux-kernel,
	Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu, stable

Hi Julian,

> On Jul 6, 2026, at 22:17, Julian Anastasov <ja@ssi.bg> wrote:
> 
> 
> Hello,
> 
> On Mon, 6 Jul 2026, Yizhou Zhao wrote:
> 
>>> On Jul 6, 2026, at 01:35, Julian Anastasov <ja@ssi.bg> wrote:
>>> 
>>> May be it is better starting from ip_vs_set_state()
>>> to provide new arg 'int iph_len/offset' (set to iph.len), down to
>>> state_transition(), sctp_state_transition() and set_sctp_state().
>>> Same for all protos. It should cost less stack and ipv6_find_hdr()
>>> calls and what matters most, correct iph context in case we
>>> have IP+ICMP+TCP (with just two ports or even with TCP flags)
>>> and are scheduling ICMP, i.e. not IP+TCP as usually.
>> 
>> I agree that the already parsed transport-header offset should be 
>> passed from ip_vs_set_state() down to the protocol state_transition() 
>> callbacks, instead of reparsing the skb in set_sctp_state(). We will 
>> send a v2 that does this for SCTP, TCP and the other IPVS protocols 
>> in one combined fix.
>> 
>>> But what I see is that ip_vs_in_icmp*() are missing
>>> the ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd) call just
>>> after ip_vs_in_stats() and before ip_vs_icmp_xmit() where
>>> we should provide ciph.len. That is why we don't reach the
>>> set_tcp_state() calls to set correct cp->state and timeout
>>> when scheduling related ICMP. So, this should be fixed too.
>> 
>> For the ICMP path, I agree that the missing ip_vs_set_state() call is
>> worth looking at, but using ICMP errors to drive the upper L4 state 
>> needs some care, because spoofed ICMP packets can match an 
>> existing embedded tuple before the endpoint TCP/SCTP stack 
>> performs its own validation. Maybe this change needs further
>> discussion?
> 
> May be only for the schedule_icmp case while the other
> ICMP replies should not change it:
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 7f93239898ff..05fdcf4ce2c0 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1971,6 +1971,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
> 
> /* do the statistics and put it back */
> ip_vs_in_stats(cp, skb);
> + if (new_cp)
> + ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd, offset);
> if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol ||
>    IPPROTO_SCTP == cih->protocol)
> offset += 2 * sizeof(__u16);
> 
> Here is why schedule_icmp was added:
> 
> https://archive.linuxvirtualserver.org/html/lvs-devel/2015-08/msg00015.html
> 
> But the inner TCP header should be generated by the
> real server, not from the client, so things can go wrong. We can
> leave it as it is now - we will forward the ICMP to the right real 
> server by using short conn timeout...

I agree. The embedded TCP/SCTP header in this path may not have the same
meaning as a normal client packet reaching the virtual service, so I do not
see a good fix for the ICMP state update part at the moment.

BTW, we have kept the ICMP handling unchanged and submitted a v2 series
for the parsed transport-offset issue only, following your earlier suggestion
to pass the already parsed offset from ip_vs_set_state() down to the protocol
state handlers. The v2 also puts the TCP and SCTP fixes in the same thread:

https://lore.kernel.org/netdev/20260706101624.69471-1-zhaoyz24@mails.tsinghua.edu.cn/

Thanks for the review.

> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>

Thanks,
Yizhou

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

end of thread, other threads:[~2026-07-07  5:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-05 12:30 [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup Yizhou Zhao
2026-07-05 17:35 ` Julian Anastasov
2026-07-06  8:22   ` Yizhou Zhao
2026-07-06 14:17     ` Julian Anastasov
2026-07-07  5:38       ` Yizhou Zhao

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