lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231223211306.GA215659@kernel.org> Date: Sat, 23 Dec 2023 21:13:06 +0000 From: Simon Horman <horms@...nel.org> To: Brad Cowie <brad@...cet.nz> Cc: netdev@...r.kernel.org, dev@...nvswitch.org, fw@...len.de, linux-kernel@...r.kernel.org, kadlec@...filter.org, edumazet@...gle.com, netfilter-devel@...r.kernel.org, kuba@...nel.org, pabeni@...hat.com, davem@...emloft.net, pablo@...filter.org, Xin Long <lucien.xin@...il.com>, Aaron Conole <aconole@...hat.com>, coreteam@...filter.org Subject: Re: [PATCH net] netfilter: nf_nat: fix action not being set for all ct states + Xin Long <lucien.xin@...il.com> Aaron Conole <aconole@...hat.com> coreteam@...filter.org On Fri, Dec 22, 2023 at 11:43:11AM +1300, Brad Cowie wrote: > This fixes openvswitch's handling of nat packets in the related state. > > In nf_ct_nat_execute(), which is called from nf_ct_nat(), ICMP/ICMPv6 > packets in the IP_CT_RELATED or IP_CT_RELATED_REPLY state, which have > not been dropped, will follow the goto, however the placement of the > goto label means that updating the action bit field will be bypassed. > > This causes ovs_nat_update_key() to not be called from ovs_ct_nat() > which means the openvswitch match key for the ICMP/ICMPv6 packet is not > updated and the pre-nat value will be retained for the key, which will > result in the wrong openflow rule being matched for that packet. > > Move the goto label above where the action bit field is being set so > that it is updated in all cases where the packet is accepted. > > Fixes: ebddb1404900 ("net: move the nat function to nf_nat_ovs for ovs and tc") > Signed-off-by: Brad Cowie <brad@...cet.nz> Thanks Brad, I agree with your analysis and that the problem appears to have been introduced by the cited commit. I am curious to know what use case triggers this / why it when unnoticed for a year. But in any case, this fix looks good to me. Reviewed-by: Simon Horman <horms@...nel.org> > --- > net/netfilter/nf_nat_ovs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/nf_nat_ovs.c b/net/netfilter/nf_nat_ovs.c > index 551abd2da614..0f9a559f6207 100644 > --- a/net/netfilter/nf_nat_ovs.c > +++ b/net/netfilter/nf_nat_ovs.c > @@ -75,9 +75,10 @@ static int nf_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct, > } > > err = nf_nat_packet(ct, ctinfo, hooknum, skb); > +out: > if (err == NF_ACCEPT) > *action |= BIT(maniptype); > -out: > + > return err; > } > > -- > 2.34.1 > > _______________________________________________ > dev mailing list > dev@...nvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Powered by blists - more mailing lists