| Message ID | 20260927220816.268206-3-pablo@netfilter.org |
|---|---|
| State | New |
| Headers | show |
| Series | [net,01/11] netfilter: ipset: do not update comments from kernel-side adds | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 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 commit message for "ipvs: fix buffer overflow when sending sync messages" says the SEQ_MASK race "should not be reproducible",… Pre-existing issues: - [Medium] After the patch, the flags are a consistent snapshot, but the sequence data is still read without locking and without ordering… -- ipvs: fix buffer overflow when sending sync messages This commit changes ip_vs_sync_conn_v0() and ip_vs_sync_conn() to read cp->flags once with READ_ONCE(). The same value is then used to size the sync entry and to decide whether to emit the sequence options. This prevents a heap overflow when another CPU sets IP_VS_CONN_F_SEQ_MASK between the two reads. Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg > Note that IPVS does not set the SEQ_MASK anymore for the > ip_vs_ftp.c helper starting from commit 7f1c40757951 > ("IPVS: make FTP work with full NAT support") (2.6.36+), > so the problem should not be reproducible. [Severity: Medium] Is the "should not be reproducible" part accurate? The ip_vs_ftp part is true. ip_vs_ftp_out() and ip_vs_ftp_in() always set *diff = 0, so vs_seq_update() is never reached. However, the SEQ bits still seem to have another writer, and received sync messages drive it. In ip_vs_proc_conn(), an existing connection has its flags updated under cp->lock: net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() { ... flags &= IP_VS_CONN_F_BACKUP_UPD_MASK; flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK; cp->flags = flags; ... } IP_VS_CONN_F_BACKUP_UPD_MASK includes the SEQ bits: #define IP_VS_CONN_F_BACKUP_UPD_MASK (IP_VS_CONN_F_INACTIVE | \ IP_VS_CONN_F_SEQ_MASK) ip_vs_proc_sync_conn() masks the wire flags with IP_VS_CONN_F_BACKUP_MASK, which also keeps the SEQ bits. It does not require an IPVS_OPT_SEQ_DATA option to be present. So a peer's sync message can set or clear the SEQ bits on an existing connection. start_sync_thread() checks the master and backup roles separately and then does ipvs->sync_state |= state, so one node can run both daemons. On such a node: CPU1 (backup kthread) ip_vs_process_message() ip_vs_proc_sync_conn() ip_vs_proc_conn() cp->flags = flags; /* SEQ bits toggled */ CPU2 (packet path, or ip_vs_conn_expire()) ip_vs_in() ip_vs_sync_conn() reads cp->flags to compute len, then again for IPVS_OPT_SEQ_DATA Before this patch, that double read could size len without the options. It could then write struct ip_vs_sync_conn_options past buff->end. Sync traffic is unauthenticated; a later patch in this series calls it not secure by design. Also, "ipvs: filter some flags received in the backup server" does not filter the SEQ bits, so this writer is still live at the end of the series. Could the commit message drop the "should not be reproducible" claim and mention the backup update path? The code change itself does look like it closes the overflow. > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c > index 5383aeafb0aed..dfa8487ec0c20 100644 > --- a/net/netfilter/ipvs/ip_vs_sync.c > +++ b/net/netfilter/ipvs/ip_vs_sync.c [ ... ] > @@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) > sloop: > if (!ip_vs_sync_conn_needed(ipvs, cp, pkts)) > goto control; > + flags = READ_ONCE(cp->flags); > > /* Sanity checks */ > pe_name_len = 0; [ ... ] > @@ -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. vs_seq_update() in ip_vs_app.c is another writer. It updates the sequence data and flags under cp->lock, but the sync reader never takes that lock. In-tree it is only reached with diff != 0, which ip_vs_ftp never produces. This does not corrupt memory, because the buffer size and the option emission now both come from the same snapshot.
Hello, On Mon, 28 Sep 2026, netdev-bot+sashiko@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential > issue(s) to consider. > > Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 > > 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 commit message for "ipvs: fix buffer overflow when sending > sync messages" says the SEQ_MASK race "should not be reproducible",… > > Pre-existing issues: > - [Medium] After the patch, the flags are a consistent snapshot, but the > sequence data is still read without locking and without ordering… > > -- > > ipvs: fix buffer overflow when sending sync messages > > This commit changes ip_vs_sync_conn_v0() and ip_vs_sync_conn() to read > cp->flags once with READ_ONCE(). The same value is then used to size the > sync entry and to decide whether to emit the sequence options. This > prevents a heap overflow when another CPU sets IP_VS_CONN_F_SEQ_MASK > between the two reads. > > Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org > Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg > > > Note that IPVS does not set the SEQ_MASK anymore for the > > ip_vs_ftp.c helper starting from commit 7f1c40757951 > > ("IPVS: make FTP work with full NAT support") (2.6.36+), > > so the problem should not be reproducible. > > [Severity: Medium] > Is the "should not be reproducible" part accurate? > > The ip_vs_ftp part is true. ip_vs_ftp_out() and ip_vs_ftp_in() always set > *diff = 0, so vs_seq_update() is never reached. However, the SEQ bits > still seem to have another writer, and received sync messages drive it. > > In ip_vs_proc_conn(), an existing connection has its flags updated under > cp->lock: > > net/netfilter/ipvs/ip_vs_sync.c:ip_vs_proc_conn() { > ... > flags &= IP_VS_CONN_F_BACKUP_UPD_MASK; > flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK; > cp->flags = flags; > ... > } > > IP_VS_CONN_F_BACKUP_UPD_MASK includes the SEQ bits: > > #define IP_VS_CONN_F_BACKUP_UPD_MASK (IP_VS_CONN_F_INACTIVE | \ > IP_VS_CONN_F_SEQ_MASK) > > ip_vs_proc_sync_conn() masks the wire flags with IP_VS_CONN_F_BACKUP_MASK, > which also keeps the SEQ bits. It does not require an IPVS_OPT_SEQ_DATA > option to be present. So a peer's sync message can set or clear the SEQ > bits on an existing connection. > > start_sync_thread() checks the master and backup roles separately and then > does ipvs->sync_state |= state, so one node can run both daemons. On such > a node: > > CPU1 (backup kthread) > ip_vs_process_message() > ip_vs_proc_sync_conn() > ip_vs_proc_conn() > cp->flags = flags; /* SEQ bits toggled */ > > CPU2 (packet path, or ip_vs_conn_expire()) > ip_vs_in() > ip_vs_sync_conn() > reads cp->flags to compute len, then again for IPVS_OPT_SEQ_DATA > > Before this patch, that double read could size len without the options. > It could then write struct ip_vs_sync_conn_options past buff->end. > > Sync traffic is unauthenticated; a later patch in this series calls it > not secure by design. Also, "ipvs: filter some flags received in the > backup server" does not filter the SEQ bits, so this writer is still > live at the end of the series. > > Could the commit message drop the "should not be reproducible" claim and > mention the backup update path? The code change itself does look like it > closes the overflow. > > > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c > > index 5383aeafb0aed..dfa8487ec0c20 100644 > > --- a/net/netfilter/ipvs/ip_vs_sync.c > > +++ b/net/netfilter/ipvs/ip_vs_sync.c > > [ ... ] > > > @@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) > > sloop: > > if (!ip_vs_sync_conn_needed(ipvs, cp, pkts)) > > goto control; > > + flags = READ_ONCE(cp->flags); > > > > /* Sanity checks */ > > pe_name_len = 0; > > [ ... ] > > > @@ -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. > vs_seq_update() in ip_vs_app.c is another writer. It updates the sequence > data and flags under cp->lock, but the sync reader never takes that lock. > In-tree it is only reached with diff != 0, which ip_vs_ftp never > produces. > > This does not corrupt memory, because the buffer size and the option > emission now both come from the same snapshot. > > -- > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927220816.268206-2-pablo%40netfilter.org Regards -- Julian Anastasov <ja@ssi.bg>
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. /P
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.
On 9/29/26 11:43, Pablo Neira Ayuso wrote: > 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. Uhm... I must admit that given the pressure we received from Linus on shrinking/prevent from increasing the net PR size I think we are better off dropping patch 2/11 entirely. I'm sorry for the extra work, but the messaging from Linus was pretty strong: https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/ Thanks, Paolo
On Tue, Sep 29, 2026 at 11:55:11AM +0200, Paolo Abeni wrote: > On 9/29/26 11:43, Pablo Neira Ayuso wrote: > > 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. > > Uhm... I must admit that given the pressure we received from Linus on > shrinking/prevent from increasing the net PR size I think we are better > off dropping patch 2/11 entirely. > > I'm sorry for the extra work, but the messaging from Linus was pretty > strong: > > https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/ Yes, I read that. I tried to move what is less relevant to nf-next (if you look at my nf-next PR, it's contains not so relevant fixes too). include/linux/netdevice.h | 3 ++ include/net/netfilter/nf_flow_table.h | 2 +- net/mac80211/iface.c | 7 +++++ net/netfilter/ipset/ip_set_bitmap_gen.h | 2 +- net/netfilter/ipvs/ip_vs_conn.c | 3 ++ net/netfilter/ipvs/ip_vs_lblc.c | 4 +++ net/netfilter/ipvs/ip_vs_lblcr.c | 3 ++ net/netfilter/ipvs/ip_vs_sync.c | 56 ++++++++++++++++++++++++++------- net/netfilter/nf_flow_table_core.c | 7 ++++- net/netfilter/nf_flow_table_offload.c | 14 +++------ net/netfilter/nf_flow_table_path.c | 3 ++ net/netfilter/nf_nat_bpf.c | 3 ++ net/netfilter/nf_nat_core.c | 5 +-- net/netfilter/nft_flow_offload.c | 7 ++++- net/netfilter/nft_set_rbtree.c | 2 ++ net/sched/act_ct.c | 2 +- 16 files changed, 95 insertions(+), 28 deletions(-) Yes, this patch is larger in the diffstat. I can just move it to nf-next if you prefer. I will respin and send v2 today without this. Thanks.
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c index 5383aeafb0ae..dfa8487ec0c2 100644 --- a/net/netfilter/ipvs/ip_vs_sync.c +++ b/net/netfilter/ipvs/ip_vs_sync.c @@ -543,13 +543,15 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, struct ip_vs_sync_conn_v0 *s; struct ip_vs_sync_buff *buff; struct ipvs_master_sync_state *ms; - int id; + u32 flags, seq_mask; unsigned int len; + int id; if (unlikely(cp->af != AF_INET)) return; + flags = READ_ONCE(cp->flags); /* Do not sync ONE PACKET */ - if (cp->flags & IP_VS_CONN_F_ONE_PACKET) + if (flags & IP_VS_CONN_F_ONE_PACKET) return; if (!ip_vs_sync_conn_needed(ipvs, cp, pkts)) @@ -564,8 +566,8 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, id = select_master_thread_id(ipvs, cp); ms = &ipvs->ms[id]; buff = ms->sync_buff; - len = (cp->flags & IP_VS_CONN_F_SEQ_MASK) ? FULL_CONN_SIZE : - SIMPLE_CONN_SIZE; + seq_mask = flags & IP_VS_CONN_F_SEQ_MASK; + len = seq_mask ? FULL_CONN_SIZE : SIMPLE_CONN_SIZE; if (buff) { m = (struct ip_vs_sync_mesg_v0 *) buff->mesg; /* Send buffer if it is for v1 */ @@ -597,9 +599,9 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, s->caddr = cp->caddr.ip; s->vaddr = cp->vaddr.ip; s->daddr = cp->daddr.ip; - s->flags = htons(cp->flags & ~IP_VS_CONN_F_HASHED); + s->flags = htons(flags & ~IP_VS_CONN_F_HASHED); s->state = htons(cp->state); - if (cp->flags & IP_VS_CONN_F_SEQ_MASK) { + if (seq_mask) { struct ip_vs_sync_conn_options *opt = (struct ip_vs_sync_conn_options *)&s[1]; memcpy(opt, &cp->sync_conn_opt, sizeof(*opt)); @@ -635,6 +637,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) int id; __u8 *p; unsigned int len, pe_name_len, pad; + u32 flags, seq_mask; /* Handle old version of the protocol */ if (sysctl_sync_ver(ipvs) == 0) { @@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) sloop: if (!ip_vs_sync_conn_needed(ipvs, cp, pkts)) goto control; + flags = READ_ONCE(cp->flags); /* Sanity checks */ pe_name_len = 0; @@ -674,7 +678,8 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) #endif len = sizeof(struct ip_vs_sync_v4); - if (cp->flags & IP_VS_CONN_F_SEQ_MASK) + seq_mask = flags & IP_VS_CONN_F_SEQ_MASK; + if (seq_mask) len += sizeof(struct ip_vs_sync_conn_options) + 2; if (cp->pe_data_len) @@ -720,7 +725,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts) /* Set message type & copy members */ s->v4.type = (cp->af == AF_INET6 ? STYPE_F_INET6 : 0); s->v4.ver_size = htons(len & SVER_MASK); /* Version 0 */ - s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED); + s->v4.flags = htonl(flags & ~IP_VS_CONN_F_HASHED); s->v4.state = htons(cp->state); s->v4.protocol = cp->protocol; s->v4.cport = cp->cport; @@ -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);