[PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[Axel Mierczuk]

A template received over the unauthenticated sync protocol with a
nonzero cport is matched by data packets in ip_vs_conn_in_get() and
the protocol state machine runs on it. Patch 1 rejects sync records
with inconsistent cport and IP_VS_CONN_F_NO_CPORT combinations on
the backup.

Patch 2 requires a nonzero cp->cport in ip_vs_conn_out_get() so that
templates and connections still waiting for their cport cannot be
matched through the out hook.

The companion patch "ipvs: filter some flags received in the backup
server" rejects template records carrying IP_VS_CONN_F_NO_CPORT.

Axel Mierczuk (2):
  ipvs: validate cport in received sync records
  ipvs: skip cport 0 connections in ip_vs_conn_out_get()

 net/netfilter/ipvs/ip_vs_conn.c |  3 ++-
 net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH nf 1/2] ipvs: validate cport in received sync records

[Axel Mierczuk]

Local templates have cport 0 and IP_VS_CONN_F_NO_CPORT clear, so
ip_vs_conn_in_get() cannot match them. A template received over the
unauthenticated sync protocol with a nonzero cport can reach the
protocol state machine. Before template state validation, this
could cause out-of-bounds reads, crashes and kernel data leaks.

Ordinary records with cport 0 and IP_VS_CONN_F_NO_CPORT clear, or
with a nonzero cport and IP_VS_CONN_F_NO_CPORT set, create connections
that incoming lookups cannot match. The latter also keeps the
cport-zero fallback in ip_vs_conn_in_get() enabled.

Reject all three cases. Continue accepting ordinary connections
with cport 0 and IP_VS_CONN_F_NO_CPORT. The companion patch
"ipvs: filter some flags received in the backup server" rejects
template records carrying IP_VS_CONN_F_NO_CPORT.

Fixes: 87375ab47cd0 ("[IPVS]: ip_vs_ftp breaks connections using persistence")
Cc: stable@vger.kernel.org
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Axel Mierczuk <axel.mierczuk@1password.com>
---
 net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 5383aeafb0ae..3998e9a0a391 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -848,6 +848,25 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
 	struct ip_vs_dest *dest;
 	struct ip_vs_conn *cp;
 
+	/* Templates require cport 0. Ordinary connections require
+	 * NO_CPORT exactly when cport is 0.
+	 */
+	if (flags & IP_VS_CONN_F_TEMPLATE) {
+		if (param->cport) {
+			IP_VS_DBG(2, "BACKUP, template with cport dropped\n");
+			kfree(param->pe_data);
+			return;
+		}
+	} else if (!param->cport && !(flags & IP_VS_CONN_F_NO_CPORT)) {
+		IP_VS_DBG(2, "BACKUP, conn without cport dropped\n");
+		kfree(param->pe_data);
+		return;
+	} else if (param->cport && (flags & IP_VS_CONN_F_NO_CPORT)) {
+		IP_VS_DBG(2, "BACKUP, conn with cport and NO_CPORT dropped\n");
+		kfree(param->pe_data);
+		return;
+	}
+
 	if (!(flags & IP_VS_CONN_F_TEMPLATE)) {
 		cp = ip_vs_conn_in_get(param);
 		if (cp && ((cp->dport != dport) ||
-- 
2.43.0

Re: [PATCH nf 1/2] ipvs: validate cport in received sync records

[netdev-bot+sashiko]

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

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

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).

- [Medium] The new check in ip_vs_proc_conn()
  (net/netfilter/ipvs/ip_vs_sync.c) is incomplete for templates.
- [Low] The new comment and rejection in ip_vs_proc_conn() say ordinary
  connections must have NO_CPORT exactly when cport is 0, and records…

--

ipvs: validate cport in received sync records

This patch adds cport checks to ip_vs_proc_conn() for sync records
received by the backup. Templates with a nonzero cport are dropped.
Ordinary connections are dropped unless IP_VS_CONN_F_NO_CPORT is set
exactly when cport is 0.

> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 5383aeafb0aed..3998e9a0a391c 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -848,6 +848,25 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
>  	struct ip_vs_dest *dest;
>  	struct ip_vs_conn *cp;
>  
> +	/* Templates require cport 0. Ordinary connections require
> +	 * NO_CPORT exactly when cport is 0.
> +	 */
> +	if (flags & IP_VS_CONN_F_TEMPLATE) {
> +		if (param->cport) {
> +			IP_VS_DBG(2, "BACKUP, template with cport dropped\n");
> +			kfree(param->pe_data);
> +			return;
> +		}

[Severity: Medium]
Should the template branch also reject IP_VS_CONN_F_NO_CPORT?

A template with cport 0 and IP_VS_CONN_F_NO_CPORT set passes this check.
The commit message leaves that case to "ipvs: filter some flags received
in the backup server". That patch is not in this tree, and this one is
tagged for stable.

In this tree, IP_VS_CONN_F_BACKUP_MASK still includes both NO_CPORT and
TEMPLATE, so ip_vs_proc_sync_conn() lets the combination through. The v0
parser applies no mask at all:

net/netfilter/ipvs/ip_vs_sync.c:ip_vs_process_message_v0() {
    ...
		flags = ntohs(s->flags) | IP_VS_CONN_F_SYNC;
		flags &= ~IP_VS_CONN_F_HASHED;
    ...
}

A companion fix that only changes IP_VS_CONN_F_BACKUP_MASK would
therefore not cover v0 records.

If ip_vs_ct_in_get() misses, ip_vs_conn_new() hashes the template and
turns on the cport-zero fallback:

net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_new() {
    ...
	if (unlikely(flags & IP_VS_CONN_F_NO_CPORT)) {
		int af_id = ip_vs_af_index(cp->af);

		atomic_inc(&ipvs->no_cport_conns[af_id]);
	}
    ...
}

A later packet from that client, from any source port, misses the first
lookup. ip_vs_conn_in_get() then retries with cport 0. The match in
__ip_vs_conn_in_get() has no IP_VS_CONN_F_TEMPLATE exclusion, so the
retry returns the template:

				    (!p->cport ^
				     (!(cp->flags & IP_VS_CONN_F_NO_CPORT))) &&

The datapath then treats the template as an ordinary connection:

ip_vs_in_hook()
  ip_vs_conn_in_get()        /* returns the template */
  ip_vs_set_state()
  cp->packet_xmit()
    ip_vs_conn_fill_cport()  /* NAT: rewrites cport, clears NO_CPORT */

After fill_cport rehashes the template, ip_vs_ct_in_get() no longer finds
it, which breaks persistence for that client.

set_tcp_state() also changes dest->activeconns for the template, based
only on IP_VS_CONN_F_INACTIVE. For templates, ip_vs_bind_dest() and
ip_vs_unbind_dest() only adjust persistconns, so activeconns on the real
server can drift.

Template state is validated now, so this does not seem to lead to
out-of-bounds reads. Still, isn't this the same class of problem the
commit message describes, a template reaching the protocol state machine
through ip_vs_conn_in_get()?

Would adding "|| (flags & IP_VS_CONN_F_NO_CPORT)" to the template check
make this patch self-contained and cover the v0 path as well?

> +	} else if (!param->cport && !(flags & IP_VS_CONN_F_NO_CPORT)) {
> +		IP_VS_DBG(2, "BACKUP, conn without cport dropped\n");
> +		kfree(param->pe_data);
> +		return;
> +	} else if (param->cport && (flags & IP_VS_CONN_F_NO_CPORT)) {
> +		IP_VS_DBG(2, "BACKUP, conn with cport and NO_CPORT dropped\n");
> +		kfree(param->pe_data);
> +		return;
> +	}

[Severity: Low]
Can the master itself briefly send a record with a nonzero cport and
IP_VS_CONN_F_NO_CPORT still set?

ip_vs_conn_fill_cport() can use hn1 (use2, e.g. NAT/FTP data
connections). In that case the dir 1 pass sets cport while the flag stays
set. cp->lock is then released, and only the dir 0 pass clears the flag:

net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_fill_cport() {
    ...
	if (cp->flags & IP_VS_CONN_F_NO_CPORT && (!cp->cport || by_me)) {
		/* If we race with resizing make sure cport is set for dir 1 */
		if (!cp->cport) {
			cp->cport = cport;
			by_me = true;
		}
		if (!dir) {
			atomic_dec(&ipvs->no_cport_conns[af_id]);
			cp->flags &= ~IP_VS_CONN_F_NO_CPORT;
		}
    ...
}

In that window, another CPU can handle a packet of the same connection
and call ip_vs_sync_conn(). That function copies cp->flags and cp->cport
without taking cp->lock:

net/netfilter/ipvs/ip_vs_sync.c:ip_vs_sync_conn() {
    ...
	s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED);
	s->v4.state = htons(cp->state);
	s->v4.protocol = cp->protocol;
	s->v4.cport = cp->cport;
    ...
}

Before this patch, the backup matched such a record to its existing
cport 0 NO_CPORT connection through the cport-zero fallback. It then
updated that connection's INACTIVE/SEQ flags and seq options. Now the
record is dropped.

The next sync record restores the state, so the effect looks small. Is
the comment "NO_CPORT exactly when cport is 0" accurate, given what the
master can send in this window?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

Re: [PATCH nf 1/2] ipvs: validate cport in received sync records

[Julian Anastasov]

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


	Hello,

On Tue, 29 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> 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).
> 
> - [Medium] The new check in ip_vs_proc_conn()
>   (net/netfilter/ipvs/ip_vs_sync.c) is incomplete for templates.
> - [Low] The new comment and rejection in ip_vs_proc_conn() say ordinary
>   connections must have NO_CPORT exactly when cport is 0, and records…

	The patch is fine. I'll reply to both concerns below.

> --
> 
> ipvs: validate cport in received sync records
> 
> This patch adds cport checks to ip_vs_proc_conn() for sync records
> received by the backup. Templates with a nonzero cport are dropped.
> Ordinary connections are dropped unless IP_VS_CONN_F_NO_CPORT is set
> exactly when cport is 0.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 5383aeafb0aed..3998e9a0a391c 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -848,6 +848,25 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
> >  	struct ip_vs_dest *dest;
> >  	struct ip_vs_conn *cp;
> >  
> > +	/* Templates require cport 0. Ordinary connections require
> > +	 * NO_CPORT exactly when cport is 0.
> > +	 */
> > +	if (flags & IP_VS_CONN_F_TEMPLATE) {
> > +		if (param->cport) {
> > +			IP_VS_DBG(2, "BACKUP, template with cport dropped\n");
> > +			kfree(param->pe_data);
> > +			return;
> > +		}
> 
> [Severity: Medium]
> Should the template branch also reject IP_VS_CONN_F_NO_CPORT?
> 
> A template with cport 0 and IP_VS_CONN_F_NO_CPORT set passes this check.
> The commit message leaves that case to "ipvs: filter some flags received
> in the backup server". That patch is not in this tree, and this one is
> tagged for stable.
> 
> In this tree, IP_VS_CONN_F_BACKUP_MASK still includes both NO_CPORT and
> TEMPLATE, so ip_vs_proc_sync_conn() lets the combination through. The v0
> parser applies no mask at all:
> 
> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_process_message_v0() {
>     ...
> 		flags = ntohs(s->flags) | IP_VS_CONN_F_SYNC;
> 		flags &= ~IP_VS_CONN_F_HASHED;
>     ...
> }
> 
> A companion fix that only changes IP_VS_CONN_F_BACKUP_MASK would
> therefore not cover v0 records.
> 
> If ip_vs_ct_in_get() misses, ip_vs_conn_new() hashes the template and
> turns on the cport-zero fallback:
> 
> net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_new() {
>     ...
> 	if (unlikely(flags & IP_VS_CONN_F_NO_CPORT)) {
> 		int af_id = ip_vs_af_index(cp->af);
> 
> 		atomic_inc(&ipvs->no_cport_conns[af_id]);
> 	}
>     ...
> }
> 
> A later packet from that client, from any source port, misses the first
> lookup. ip_vs_conn_in_get() then retries with cport 0. The match in
> __ip_vs_conn_in_get() has no IP_VS_CONN_F_TEMPLATE exclusion, so the
> retry returns the template:
> 
> 				    (!p->cport ^
> 				     (!(cp->flags & IP_VS_CONN_F_NO_CPORT))) &&
> 
> The datapath then treats the template as an ordinary connection:
> 
> ip_vs_in_hook()
>   ip_vs_conn_in_get()        /* returns the template */
>   ip_vs_set_state()
>   cp->packet_xmit()
>     ip_vs_conn_fill_cport()  /* NAT: rewrites cport, clears NO_CPORT */
> 
> After fill_cport rehashes the template, ip_vs_ct_in_get() no longer finds
> it, which breaks persistence for that client.
> 
> set_tcp_state() also changes dest->activeconns for the template, based
> only on IP_VS_CONN_F_INACTIVE. For templates, ip_vs_bind_dest() and
> ip_vs_unbind_dest() only adjust persistconns, so activeconns on the real
> server can drift.
> 
> Template state is validated now, so this does not seem to lead to
> out-of-bounds reads. Still, isn't this the same class of problem the
> commit message describes, a template reaching the protocol state machine
> through ip_vs_conn_in_get()?
> 
> Would adding "|| (flags & IP_VS_CONN_F_NO_CPORT)" to the template check
> make this patch self-contained and cover the v0 path as well?

	No, "ipvs: filter some flags received in the backup server"
should handle the above problems.

> 
> > +	} else if (!param->cport && !(flags & IP_VS_CONN_F_NO_CPORT)) {
> > +		IP_VS_DBG(2, "BACKUP, conn without cport dropped\n");
> > +		kfree(param->pe_data);
> > +		return;
> > +	} else if (param->cport && (flags & IP_VS_CONN_F_NO_CPORT)) {
> > +		IP_VS_DBG(2, "BACKUP, conn with cport and NO_CPORT dropped\n");
> > +		kfree(param->pe_data);
> > +		return;
> > +	}
> 
> [Severity: Low]
> Can the master itself briefly send a record with a nonzero cport and
> IP_VS_CONN_F_NO_CPORT still set?
> 
> ip_vs_conn_fill_cport() can use hn1 (use2, e.g. NAT/FTP data
> connections). In that case the dir 1 pass sets cport while the flag stays
> set. cp->lock is then released, and only the dir 0 pass clears the flag:
> 
> net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_fill_cport() {
>     ...
> 	if (cp->flags & IP_VS_CONN_F_NO_CPORT && (!cp->cport || by_me)) {
> 		/* If we race with resizing make sure cport is set for dir 1 */
> 		if (!cp->cport) {
> 			cp->cport = cport;
> 			by_me = true;
> 		}
> 		if (!dir) {
> 			atomic_dec(&ipvs->no_cport_conns[af_id]);
> 			cp->flags &= ~IP_VS_CONN_F_NO_CPORT;
> 		}
>     ...
> }
> 
> In that window, another CPU can handle a packet of the same connection
> and call ip_vs_sync_conn(). That function copies cp->flags and cp->cport
> without taking cp->lock:
> 
> net/netfilter/ipvs/ip_vs_sync.c:ip_vs_sync_conn() {
>     ...
> 	s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED);
> 	s->v4.state = htons(cp->state);
> 	s->v4.protocol = cp->protocol;
> 	s->v4.cport = cp->cport;
>     ...
> }
> 
> Before this patch, the backup matched such a record to its existing
> cport 0 NO_CPORT connection through the cport-zero fallback. It then
> updated that connection's INACTIVE/SEQ flags and seq options. Now the
> record is dropped.
> 
> The next sync record restores the state, so the effect looks small. Is
> the comment "NO_CPORT exactly when cport is 0" accurate, given what the
> master can send in this window?

	TCP conns before establishment are not synced by master,
so such race can not occur. And it is normal the backup to drop
inconsistent cport and NO_CPORT flag.

> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

Regards

--
Julian Anastasov <ja@ssi.bg>

[PATCH nf 2/2] ipvs: skip cport 0 connections in ip_vs_conn_out_get()

[Axel Mierczuk]

ip_vs_conn_out_get() does not filter templates. A template with
cport 0 and DR or TUN forwarding bits is matched by a packet sent
to client port 0 that traverses the out hook, and handle_response()
then runs ip_vs_set_state() and the timeout update on a connection
that must never see packets. A backup server receives such
templates over the unauthenticated sync protocol. Connections that
still wait for their cport with IP_VS_CONN_F_NO_CPORT can be
matched the same way.

Require a nonzero cp->cport for a match. Templates keep cport 0
throughout their lifetime. FTP data connections learn their client
port from incoming traffic before replies are processed.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Axel Mierczuk <axel.mierczuk@1password.com>
---
 net/netfilter/ipvs/ip_vs_conn.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 6fa3e1dc534c..24dbe34a4827 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -579,7 +579,8 @@ struct ip_vs_conn *ip_vs_conn_out_get(const struct ip_vs_conn_param *p)
 				    ip_vs_addr_equal(p->af, p->vaddr,
 						     &cp->caddr) &&
 				    ip_vs_addr_equal(p->af, p->caddr, saddr) &&
-				    p->protocol == cp->protocol) {
+				    p->protocol == cp->protocol &&
+				    cp->cport) {
 					if (__ip_vs_conn_get(cp))
 						goto out;
 				}
-- 
2.43.0

Re: [PATCH nf 2/2] ipvs: skip cport 0 connections in ip_vs_conn_out_get()

[netdev-bot+sashiko]

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

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

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).

- [Medium] The new `cp->cport` requirement turns the "update or create"
  lookup in ip_vs_ftp_out() (net/netfilter/ipvs/ip_vs_ftp.c) into dead…
- [Medium] ip_vs_new_conn_out() (net/netfilter/ipvs/ip_vs_core.c) rejects
  only `!vport || !dport`.
- [Medium] The commit message assumes only templates and NO_CPORT entries
  have cport 0, but ordinary connections can too.
- [Medium] The commit rests on the claim "Templates keep cport 0
  throughout their lifetime", which does not hold for synced templates…
- [Low] The patch adds a second, separate plain read of cp->cport in
  ip_vs_conn_out_get().

--

ipvs: skip cport 0 connections in ip_vs_conn_out_get()

This patch makes ip_vs_conn_out_get() match only connections with a
nonzero cp->cport. The goal is to stop out-direction packets sent to
client port 0 from matching persistence templates, or connections still
waiting for their client port (IP_VS_CONN_F_NO_CPORT), and then updating
their state and timeout.

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 6fa3e1dc534c3..24dbe34a48275 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -579,7 +579,8 @@ struct ip_vs_conn *ip_vs_conn_out_get(const struct ip_vs_conn_param *p)
>  				    ip_vs_addr_equal(p->af, p->vaddr,
>  						     &cp->caddr) &&
>  				    ip_vs_addr_equal(p->af, p->caddr, saddr) &&
> -				    p->protocol == cp->protocol) {
> +				    p->protocol == cp->protocol &&
> +				    cp->cport) {

[Severity: Medium]
Does this make the "update or create" lookup in ip_vs_ftp_out() in
net/netfilter/ipvs/ip_vs_ftp.c unreachable?

ip_vs_ftp_out() looks up with vport 0 on purpose, to find an existing
IP_VS_CONN_F_NO_CPORT passive data connection:

net/netfilter/ipvs/ip_vs_ftp.c:ip_vs_ftp_out() {
    ...
	/* Now update or create a connection entry for it */
	{
		struct ip_vs_conn_param p;

		ip_vs_conn_fill_param(cp->ipvs, cp->af,
				      ipvsh->protocol, &from, port,
				      &cp->caddr, 0, &p);
		n_cp = ip_vs_conn_out_get(&p);
	}
	if (!n_cp) {
		...
		n_cp = ip_vs_conn_new(&p, cp->af, &from, port,
				      IP_VS_CONN_F_NO_CPORT |
				      IP_VS_CONN_F_NFCT,
				      cp->dest, skb->mark);
		if (!n_cp)
			return 0;
    ...
}

When p->vport is 0, the earlier check in ip_vs_conn_out_get():

	if (p->vport != cp->cport)
		continue;

only lets through entries with cp->cport == 0. The new cp->cport test
then rejects every one of them, so this lookup can never return a match.

Suppose a client sends PASV/EPSV again and the server answers with the
same passive port, for example because it has a fixed or narrow passive
range. Each such reply now creates a duplicate NO_CPORT entry with the
same hash key.

Each duplicate increments no_cport_conns[], takes another n_control
reference on the control connection and adds another conntrack
expectation. Only one of them ever gets its cport filled. The rest stay
until the listen timeout.

If conn_count has reached conn_max, ip_vs_conn_new() returns NULL and
ip_vs_ftp_out() returns 0. The control reply is dropped, where before
the existing entry would have been reused.

A retransmitted 227/229 segment does not seem to trigger this, because
cp->app_data is set to IP_VS_FTP_ACTIVE after the first pass. A repeated
PASV/EPSV command does trigger it.

The commit message says "FTP data connections learn their client port
from incoming traffic before replies are processed". Doesn't that only
cover replies on the data connection? This control-channel lookup runs
before the port is known.

[Severity: Medium]
Can this lead to a new connection being created for every packet of a
flow started by a real server towards client port 0?

ip_vs_new_conn_out() in net/netfilter/ipvs/ip_vs_core.c only rejects a
zero vport or dport:

	if (!vport || !dport)
		return NULL;

Take a SIP-PE service with conn_out_counter > 0, where a real server
sends a UDP packet to client port 0. This path:

ip_vs_out_hook()->__ip_vs_rs_conn_out()->ip_vs_sip_conn_out()->
    ip_vs_new_conn_out()->ip_vs_conn_new()

creates an ordinary hashed connection with cp->cport == 0 and neither
TEMPLATE nor NO_CPORT set.

Before this patch, the next packet of that flow matched this entry in
ip_vs_conn_out_get(). With the new cp->cport test it misses. So
ip_vs_out_hook() falls back to __ip_vs_rs_conn_out() again and creates
yet another connection.

Each extra connection lives until the UDP timeout. For persistent
services, each one also takes another n_control reference on the
template.

__ip_vs_conn_in_get() cannot match these entries either, because a
cport 0 lookup there requires NO_CPORT.

Linux UDP sockets refuse sendto() to port 0, so in practice this needs a
raw socket or a non-Linux real server.

Should ip_vs_new_conn_out() also reject a zero cport? Or should the out
lookup exclude only templates and NO_CPORT entries?

[Severity: Medium]
The commit message assumes that only templates and NO_CPORT entries have
cport 0. Does that hold for ordinary MASQ connections scheduled for a
client that uses source port 0?

ip_vs_schedule() in net/netfilter/ipvs/ip_vs_core.c takes the client
port directly from the packet:

	cport = pptr[0];
	...
	ip_vs_conn_fill_param(svc->ipvs, svc->af, iph->protocol,
			      caddr, cport, vaddr, vport, &p);
	cp = ip_vs_conn_new(&p, dest->af, &dest->addr,
			    dest->port ? dest->port : vport,
			    flags, dest, skb->mark);

Nothing in the scheduling path rejects source port 0. A remote client
sending a TCP SYN or UDP packet from port 0 to VIP:vport on a
non-persistent MASQ service gets an ordinary connection with
cp->cport == 0.

Before this patch, the real server's reply to client:0 matched this
connection in ip_vs_conn_out_get() and was SNATed back to VIP:vport. Now
the lookup rejects it.

With the default nat_icmp_send=0, ip_vs_out_hook() then returns
NF_ACCEPT. The reply, for example a SYN-ACK or RST, leaves the director
with the real server's private address and port as its source.

ICMP errors behave the same way. If the real server sends an error that
quotes client:0->RS:rport, ip_vs_out_icmp() does the reversed lookup
with vport 0:

	cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto,
			     ipvs, AF_INET, skb, &ciph);
	if (!cp)
		return NF_ACCEPT;

This now returns NULL, so neither the outer source nor the embedded
tuple is reverse-NATed.

__ip_vs_conn_in_get() already could not match such entries. The out
direction, though, was NATed before this patch.

Would excluding IP_VS_CONN_F_TEMPLATE (and NO_CPORT, if intended) by
flag avoid this?

[Severity: Low]
Can the two separate plain reads of cp->cport in ip_vs_conn_out_get()
see different values?

The lookup holds only rcu_read_lock(). Meanwhile, ip_vs_conn_fill_cport()
writes cp->cport under cp->lock and the bucket locks, before it updates
hn->hash_key and moves the node:

	if (!cp->cport) {
		cp->cport = cport;
		by_me = true;
	}

It can also reset cp->cport to 0 on the "Too many ht changes" path.

Suppose a MASQ NO_CPORT connection is getting its cport filled while an
out-direction packet to client port 0 is looked up. The reader can see
the old hash_key and read cp->cport == 0 in:

	if (p->vport != cp->cport)
		continue;

and then read the new nonzero port in the added cp->cport test. The
entry then matches, and handle_response() runs ip_vs_set_state() and the
timeout update. That is the NO_CPORT case the commit message says this
patch closes.

A successful match leaves the loop through goto out, so the seqcount
retry in ip_vs_rht_for_bucket_retry() is never checked.

Neither side uses READ_ONCE()/WRITE_ONCE(), so KCSAN could also report
this as a data race.

Would a single snapshot close this window?

	__be16 cport = READ_ONCE(cp->cport);

	if (!cport || p->vport != cport)
		continue;

[Severity: Medium]
The commit message says:

    Templates keep cport 0 throughout their lifetime.

Is that true for synced templates that also carry IP_VS_CONN_F_NO_CPORT?

IP_VS_CONN_F_BACKUP_MASK includes both TEMPLATE and NO_CPORT. The
template check in ip_vs_proc_conn() in net/netfilter/ipvs/ip_vs_sync.c
only rejects a nonzero cport:

	if (flags & IP_VS_CONN_F_TEMPLATE) {
		if (param->cport) {
			IP_VS_DBG(2, "BACKUP, template with cport dropped\n");
			kfree(param->pe_data);
			return;
		}
	}

So a forged TEMPLATE|NO_CPORT|MASQ record with cport 0 is accepted. It
is hashed through hn0, counted in no_cport_conns[] and bound to
ip_vs_nat_xmit().

An incoming packet C:X -> vaddr:vport then goes through:

ip_vs_in_hook()
  ip_vs_conn_in_get()     <- the cport 0 fallback matches; TEMPLATE
                             is not excluded
  ip_vs_nat_xmit()
    ip_vs_conn_fill_cport(cp, X)

ip_vs_conn_fill_cport() sets cp->cport = X and clears NO_CPORT, but
leaves TEMPLATE set. It also rehashes hn0 to hash(C, X, vaddr, vport).
All of this happens before the later "stopping DNAT to local address"
check in ip_vs_nat_xmit().

Now suppose the forged record also sets daddr == vaddr and
dport == vport. An out packet vaddr:vport -> C:X then passes the
hash_key check, the MASQ saddr/sport comparisons and the new cp->cport
test. handle_response() then runs ip_vs_set_state() and the timeout
update on the template.

One way to produce that out packet: vaddr is local to the backup and
the incoming packet has no conntrack entry. The packet is then delivered
locally, and the local reply or RST goes through ip_vs_out_hook() at
LOCAL_OUT.

The in-direction fallback already let such a forged template see
packets before this patch. The out-direction match on the template is
what reappears here.

Would it be more robust to exclude IP_VS_CONN_F_TEMPLATE explicitly in
ip_vs_conn_out_get(), and to reject TEMPLATE|NO_CPORT records in
ip_vs_proc_conn()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

Re: [PATCH nf 2/2] ipvs: skip cport 0 connections in ip_vs_conn_out_get()

[Julian Anastasov]

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


	Hello,

On Tue, 29 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 4 · Low: 1
> 
> 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).
> 
> - [Medium] The new `cp->cport` requirement turns the "update or create"
>   lookup in ip_vs_ftp_out() (net/netfilter/ipvs/ip_vs_ftp.c) into dead…
> - [Medium] ip_vs_new_conn_out() (net/netfilter/ipvs/ip_vs_core.c) rejects
>   only `!vport || !dport`.
> - [Medium] The commit message assumes only templates and NO_CPORT entries
>   have cport 0, but ordinary connections can too.
> - [Medium] The commit rests on the claim "Templates keep cport 0
>   throughout their lifetime", which does not hold for synced templates…
> - [Low] The patch adds a second, separate plain read of cp->cport in
>   ip_vs_conn_out_get().

	There are valid concerns, so we should send new version.

> 
> --
> 
> ipvs: skip cport 0 connections in ip_vs_conn_out_get()
> 
> This patch makes ip_vs_conn_out_get() match only connections with a
> nonzero cp->cport. The goal is to stop out-direction packets sent to
> client port 0 from matching persistence templates, or connections still
> waiting for their client port (IP_VS_CONN_F_NO_CPORT), and then updating
> their state and timeout.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 6fa3e1dc534c3..24dbe34a48275 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -579,7 +579,8 @@ struct ip_vs_conn *ip_vs_conn_out_get(const struct ip_vs_conn_param *p)
> >  				    ip_vs_addr_equal(p->af, p->vaddr,
> >  						     &cp->caddr) &&
> >  				    ip_vs_addr_equal(p->af, p->caddr, saddr) &&
> > -				    p->protocol == cp->protocol) {
> > +				    p->protocol == cp->protocol &&
> > +				    cp->cport) {
> 
> [Severity: Medium]
> Does this make the "update or create" lookup in ip_vs_ftp_out() in
> net/netfilter/ipvs/ip_vs_ftp.c unreachable?
> 
> ip_vs_ftp_out() looks up with vport 0 on purpose, to find an existing
> IP_VS_CONN_F_NO_CPORT passive data connection:
> 
> net/netfilter/ipvs/ip_vs_ftp.c:ip_vs_ftp_out() {
>     ...
> 	/* Now update or create a connection entry for it */
> 	{
> 		struct ip_vs_conn_param p;
> 
> 		ip_vs_conn_fill_param(cp->ipvs, cp->af,
> 				      ipvsh->protocol, &from, port,
> 				      &cp->caddr, 0, &p);
> 		n_cp = ip_vs_conn_out_get(&p);
> 	}
> 	if (!n_cp) {
> 		...
> 		n_cp = ip_vs_conn_new(&p, cp->af, &from, port,
> 				      IP_VS_CONN_F_NO_CPORT |
> 				      IP_VS_CONN_F_NFCT,
> 				      cp->dest, skb->mark);
> 		if (!n_cp)
> 			return 0;
>     ...
> }
> 
> When p->vport is 0, the earlier check in ip_vs_conn_out_get():
> 
> 	if (p->vport != cp->cport)
> 		continue;
> 
> only lets through entries with cp->cport == 0. The new cp->cport test
> then rejects every one of them, so this lookup can never return a match.
> 
> Suppose a client sends PASV/EPSV again and the server answers with the
> same passive port, for example because it has a fixed or narrow passive
> range. Each such reply now creates a duplicate NO_CPORT entry with the
> same hash key.
> 
> Each duplicate increments no_cport_conns[], takes another n_control
> reference on the control connection and adds another conntrack
> expectation. Only one of them ever gets its cport filled. The rest stay
> until the listen timeout.
> 
> If conn_count has reached conn_max, ip_vs_conn_new() returns NULL and
> ip_vs_ftp_out() returns 0. The control reply is dropped, where before
> the existing entry would have been reused.
> 
> A retransmitted 227/229 segment does not seem to trigger this, because
> cp->app_data is set to IP_VS_FTP_ACTIVE after the first pass. A repeated
> PASV/EPSV command does trigger it.
> 
> The commit message says "FTP data connections learn their client port
> from incoming traffic before replies are processed". Doesn't that only
> cover replies on the data connection? This control-channel lookup runs
> before the port is known.

	Yep, this is a problem, we should change the
cp->cport check with !TEMPLATE check.

> 
> [Severity: Medium]
> Can this lead to a new connection being created for every packet of a
> flow started by a real server towards client port 0?
> 
> ip_vs_new_conn_out() in net/netfilter/ipvs/ip_vs_core.c only rejects a
> zero vport or dport:
> 
> 	if (!vport || !dport)
> 		return NULL;
> 
> Take a SIP-PE service with conn_out_counter > 0, where a real server
> sends a UDP packet to client port 0. This path:
> 
> ip_vs_out_hook()->__ip_vs_rs_conn_out()->ip_vs_sip_conn_out()->
>     ip_vs_new_conn_out()->ip_vs_conn_new()
> 
> creates an ordinary hashed connection with cp->cport == 0 and neither
> TEMPLATE nor NO_CPORT set.
> 
> Before this patch, the next packet of that flow matched this entry in
> ip_vs_conn_out_get(). With the new cp->cport test it misses. So
> ip_vs_out_hook() falls back to __ip_vs_rs_conn_out() again and creates
> yet another connection.
> 
> Each extra connection lives until the UDP timeout. For persistent
> services, each one also takes another n_control reference on the
> template.
> 
> __ip_vs_conn_in_get() cannot match these entries either, because a
> cport 0 lookup there requires NO_CPORT.
> 
> Linux UDP sockets refuse sendto() to port 0, so in practice this needs a
> raw socket or a non-Linux real server.
> 
> Should ip_vs_new_conn_out() also reject a zero cport? Or should the out
> lookup exclude only templates and NO_CPORT entries?

	This can be separate fix.

> 
> [Severity: Medium]
> The commit message assumes that only templates and NO_CPORT entries have
> cport 0. Does that hold for ordinary MASQ connections scheduled for a
> client that uses source port 0?
> 
> ip_vs_schedule() in net/netfilter/ipvs/ip_vs_core.c takes the client
> port directly from the packet:
> 
> 	cport = pptr[0];
> 	...
> 	ip_vs_conn_fill_param(svc->ipvs, svc->af, iph->protocol,
> 			      caddr, cport, vaddr, vport, &p);
> 	cp = ip_vs_conn_new(&p, dest->af, &dest->addr,
> 			    dest->port ? dest->port : vport,
> 			    flags, dest, skb->mark);
> 
> Nothing in the scheduling path rejects source port 0. A remote client
> sending a TCP SYN or UDP packet from port 0 to VIP:vport on a
> non-persistent MASQ service gets an ordinary connection with
> cp->cport == 0.
> 
> Before this patch, the real server's reply to client:0 matched this
> connection in ip_vs_conn_out_get() and was SNATed back to VIP:vport. Now
> the lookup rejects it.
> 
> With the default nat_icmp_send=0, ip_vs_out_hook() then returns
> NF_ACCEPT. The reply, for example a SYN-ACK or RST, leaves the director
> with the real server's private address and port as its source.
> 
> ICMP errors behave the same way. If the real server sends an error that
> quotes client:0->RS:rport, ip_vs_out_icmp() does the reversed lookup
> with vport 0:
> 
> 	cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto,
> 			     ipvs, AF_INET, skb, &ciph);
> 	if (!cp)
> 		return NF_ACCEPT;
> 
> This now returns NULL, so neither the outer source nor the embedded
> tuple is reverse-NATed.
> 
> __ip_vs_conn_in_get() already could not match such entries. The out
> direction, though, was NATed before this patch.
> 
> Would excluding IP_VS_CONN_F_TEMPLATE (and NO_CPORT, if intended) by
> flag avoid this?

	We should fix the above problems.

> 
> [Severity: Low]
> Can the two separate plain reads of cp->cport in ip_vs_conn_out_get()
> see different values?
> 
> The lookup holds only rcu_read_lock(). Meanwhile, ip_vs_conn_fill_cport()
> writes cp->cport under cp->lock and the bucket locks, before it updates
> hn->hash_key and moves the node:
> 
> 	if (!cp->cport) {
> 		cp->cport = cport;
> 		by_me = true;
> 	}
> 
> It can also reset cp->cport to 0 on the "Too many ht changes" path.
> 
> Suppose a MASQ NO_CPORT connection is getting its cport filled while an
> out-direction packet to client port 0 is looked up. The reader can see
> the old hash_key and read cp->cport == 0 in:
> 
> 	if (p->vport != cp->cport)
> 		continue;
> 
> and then read the new nonzero port in the added cp->cport test. The
> entry then matches, and handle_response() runs ip_vs_set_state() and the
> timeout update. That is the NO_CPORT case the commit message says this
> patch closes.
> 
> A successful match leaves the loop through goto out, so the seqcount
> retry in ip_vs_rht_for_bucket_retry() is never checked.
> 
> Neither side uses READ_ONCE()/WRITE_ONCE(), so KCSAN could also report
> this as a data race.
> 
> Would a single snapshot close this window?
> 
> 	__be16 cport = READ_ONCE(cp->cport);
> 
> 	if (!cport || p->vport != cport)
> 		continue;
> 
> [Severity: Medium]
> The commit message says:
> 
>     Templates keep cport 0 throughout their lifetime.
> 
> Is that true for synced templates that also carry IP_VS_CONN_F_NO_CPORT?
> 
> IP_VS_CONN_F_BACKUP_MASK includes both TEMPLATE and NO_CPORT. The
> template check in ip_vs_proc_conn() in net/netfilter/ipvs/ip_vs_sync.c
> only rejects a nonzero cport:
> 
> 	if (flags & IP_VS_CONN_F_TEMPLATE) {
> 		if (param->cport) {
> 			IP_VS_DBG(2, "BACKUP, template with cport dropped\n");
> 			kfree(param->pe_data);
> 			return;
> 		}
> 	}
> 
> So a forged TEMPLATE|NO_CPORT|MASQ record with cport 0 is accepted. It
> is hashed through hn0, counted in no_cport_conns[] and bound to
> ip_vs_nat_xmit().
> 
> An incoming packet C:X -> vaddr:vport then goes through:
> 
> ip_vs_in_hook()
>   ip_vs_conn_in_get()     <- the cport 0 fallback matches; TEMPLATE
>                              is not excluded
>   ip_vs_nat_xmit()
>     ip_vs_conn_fill_cport(cp, X)
> 
> ip_vs_conn_fill_cport() sets cp->cport = X and clears NO_CPORT, but
> leaves TEMPLATE set. It also rehashes hn0 to hash(C, X, vaddr, vport).
> All of this happens before the later "stopping DNAT to local address"
> check in ip_vs_nat_xmit().
> 
> Now suppose the forged record also sets daddr == vaddr and
> dport == vport. An out packet vaddr:vport -> C:X then passes the
> hash_key check, the MASQ saddr/sport comparisons and the new cp->cport
> test. handle_response() then runs ip_vs_set_state() and the timeout
> update on the template.
> 
> One way to produce that out packet: vaddr is local to the backup and
> the incoming packet has no conntrack entry. The packet is then delivered
> locally, and the local reply or RST goes through ip_vs_out_hook() at
> LOCAL_OUT.
> 
> The in-direction fallback already let such a forged template see
> packets before this patch. The out-direction match on the template is
> what reappears here.
> 
> Would it be more robust to exclude IP_VS_CONN_F_TEMPLATE explicitly in
> ip_vs_conn_out_get(), and to reject TEMPLATE|NO_CPORT records in
> ip_vs_proc_conn()?

	This is fixed by "ipvs: filter some flags received in the backup 
server"

> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

pw-bot: cr

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[Julian Anastasov]

	Hello,

On Fri, 25 Sep 2026, Axel Mierczuk wrote:

> A template received over the unauthenticated sync protocol with a
> nonzero cport is matched by data packets in ip_vs_conn_in_get() and
> the protocol state machine runs on it. Patch 1 rejects sync records
> with inconsistent cport and IP_VS_CONN_F_NO_CPORT combinations on
> the backup.
> 
> Patch 2 requires a nonzero cp->cport in ip_vs_conn_out_get() so that
> templates and connections still waiting for their cport cannot be
> matched through the out hook.
> 
> The companion patch "ipvs: filter some flags received in the backup
> server" rejects template records carrying IP_VS_CONN_F_NO_CPORT.
> 
> Axel Mierczuk (2):
>   ipvs: validate cport in received sync records
>   ipvs: skip cport 0 connections in ip_vs_conn_out_get()

	Both patches look good to me, thanks!

Acked-by: Julian Anastasov <ja@ssi.bg>

> 
>  net/netfilter/ipvs/ip_vs_conn.c |  3 ++-
>  net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> -- 
> 2.43.0

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[Julian Anastasov]

	Hello,

On Fri, 25 Sep 2026, Axel Mierczuk wrote:

> A template received over the unauthenticated sync protocol with a
> nonzero cport is matched by data packets in ip_vs_conn_in_get() and
> the protocol state machine runs on it. Patch 1 rejects sync records
> with inconsistent cport and IP_VS_CONN_F_NO_CPORT combinations on
> the backup.
> 
> Patch 2 requires a nonzero cp->cport in ip_vs_conn_out_get() so that
> templates and connections still waiting for their cport cannot be
> matched through the out hook.
> 
> The companion patch "ipvs: filter some flags received in the backup
> server" rejects template records carrying IP_VS_CONN_F_NO_CPORT.
> 
> Axel Mierczuk (2):
>   ipvs: validate cport in received sync records
>   ipvs: skip cport 0 connections in ip_vs_conn_out_get()
> 
>  net/netfilter/ipvs/ip_vs_conn.c |  3 ++-
>  net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)

	Axel, this Sashiko review is shattering:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

	I'll be back with analyze how should we fix the
problems...

pw-bot: changes-requested

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[Julian Anastasov]

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


	Hi Axel,

On Wed, 30 Sep 2026, Axel Mierczuk wrote:

> Hi Julian,
> 
> Sounds good. I will take a look as well and get back to you.
> 
> On Tue, Sep 29, 2026 at 2:29 PM Julian Anastasov <ja@ssi.bg> wrote:
> 
>               Hello,
> 
>       On Fri, 25 Sep 2026, Axel Mierczuk wrote:
> 
>       > A template received over the unauthenticated sync protocol with a
>       > nonzero cport is matched by data packets in ip_vs_conn_in_get() and
>       > the protocol state machine runs on it. Patch 1 rejects sync records
>       > with inconsistent cport and IP_VS_CONN_F_NO_CPORT combinations on
>       > the backup.
>       >
>       > Patch 2 requires a nonzero cp->cport in ip_vs_conn_out_get() so that
>       > templates and connections still waiting for their cport cannot be
>       > matched through the out hook.
>       >
>       > The companion patch "ipvs: filter some flags received in the backup
>       > server" rejects template records carrying IP_VS_CONN_F_NO_CPORT.
>       >
>       > Axel Mierczuk (2):
>       >   ipvs: validate cport in received sync records
>       >   ipvs: skip cport 0 connections in ip_vs_conn_out_get()
>       >
>       >  net/netfilter/ipvs/ip_vs_conn.c |  3 ++-
>       >  net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
>       >  2 files changed, 21 insertions(+), 1 deletion(-)
> 
>               Axel, this Sashiko review is shattering:
> 
>       https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com
> 
>               I'll be back with analyze how should we fix the
>       problems...

	Here is what we have for the Sashiko review:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

Patch 1:

Q. Should the template branch also reject IP_VS_CONN_F_NO_CPORT?
A. Fixed by "ipvs: filter some flags received in the backup server"
which is already in the trees

Q. Can the master itself briefly send a record with a nonzero cport and
IP_VS_CONN_F_NO_CPORT still set?
A. Harmless

	So, nothing that needs to change the patch.

Patch 2:

Q1. Does this make the "update or create" lookup in ip_vs_ftp_out() in
net/netfilter/ipvs/ip_vs_ftp.c unreachable?
A. Looks like we need the same check in ip_vs_conn_out_get:

We should use:
	(!p->vport ^ (!(cp->flags & IP_VS_CONN_F_NO_CPORT)))

instead of the wrong cp->cport check.

	Using p->vport and not re-reading cp->cport above due to
tricks in ip_vs_conn_fill_cport(), see Q4 below.

Q2. Can this lead to a new connection being created for every packet of a
flow started by a real server towards client port 0?
A. I can provide patch to ignore packets with port 0.

Q3. The commit message assumes that only templates and NO_CPORT entries have
cport 0. Does that hold for ordinary MASQ connections scheduled for a
client that uses source port 0?
A. I can provide patch to ignore packets with port 0.

Q4. Can the two separate plain reads of cp->cport in ip_vs_conn_out_get()
see different values?
A. We can check p->vport, see Q1

Q5. TEMPLATE and NO_CPORT
A. Fixed by "ipvs: filter some flags received in the backup server"
which is already in the trees

	There is the option to create 3th patch "validate vport and dport 
in received sync records" in your patchset, so that such change can be 
propagated together:

	} else if (!param->vport) {
		...
	} else if (!dport &&
		   (flags & IP_VS_CONN_F_FWD_MASK) == IP_VS_CONN_F_MASQ) {
		...
	}

	Because it depends on your "ipvs: validate cport in received sync 
records" patch. Such change will prevent sync messages to create
connections with port 0 that can be later hit by packets with
port 0. But may be such change is not needed.

	I can provide a separate patch that will disallow connections
with 0 in cp->cport/vport created from packets. It will not conflict
with your changes.

	After correcting the check in ip_vs_conn_out_get() lookup misses
caused by client port 0 can lead to scheduling for every packet and 
creating duplicate connections. The main thing is to isolate cport 0
which for the backup server is already in your patch. The remaining part 
is to filter it in the scheduler. OTOH, the vport/dport checks are
just nice to have. The rule here is, if we do not match cport 0 when 
NO_CPORT is not set, we should also not create connections with cport 0.
As for the zero vport/dport, I'm not sure, they look harmless.
What do you think?

Regards

--
Julian Anastasov <ja@ssi.bg>