Re: [PATCH net 02/11] ipvs: fix buffer overflow when sending sync messages

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Paolo,

On Tue, Sep 29, 2026 at 10:19:55AM +0200, Paolo Abeni wrote:
> On 9/29/26 06:06, Julian Anastasov wrote:
> > > > @@ -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.

Just wrote to Jakub with a summary on the LLM report.

I would take this PR as is if it is still possible.

If you feel strong about to need to respin this PR, that's also fine
with me, just let confirm where to go.

Thanks.