On 9/29/26 06:06, Julian Anastasov wrote:
quoted
quoted
@@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
s->v4.vaddr = cp->vaddr.ip;
s->v4.daddr = cp->daddr.ip;
}
- if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
+ if (seq_mask) {
*(p++) = IPVS_OPT_SEQ_DATA;
*(p++) = sizeof(struct ip_vs_sync_conn_options);
hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. The flags
are now a consistent snapshot, but the sequence data is still read
without cp->lock. Nothing orders that read against READ_ONCE(cp->flags).
Can the master send stale or half-updated ip_vs_seq values here?
ip_vs_sync_conn() here only holds ipvs->sync_buff_lock. The same applies
to the memcpy() of cp->sync_conn_opt in ip_vs_sync_conn_v0().
On the backup side, ip_vs_proc_conn() sets the SEQ bits in cp->flags under
cp->lock and drops the lock. Only after that does it copy the sequence
data, with no lock held:
net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() {
...
cp->flags = flags;
spin_unlock_bh(&cp->lock);
...
if (opt) {
cp->in_seq = opt->in_seq;
cp->out_seq = opt->out_seq;
}
...
}
On a node running both the master and backup daemons, ip_vs_sync_conn()
could see seq_mask set and then send old or torn init_seq, delta and
previous_delta values. A peer with an app bound to that connection would
then apply the wrong TCP seq/ack adjustment after failover.
Agreed, this can be improved to take a snapshot of
flags and seqs together under lock. I'll request to drop
this version, it is not urgent to apply it.
It looks like a respin of the PR is needed, I'll drop the revision from PW.
/P