| Message ID | 1655419806-22527-1-git-send-email-bodong@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,F:linux-bluefield] net/sched: act_ct: fix ref leak when switching zones | expand |
Acked-by: Zachary Tahenakos <zachary.tahenakos@canonical.com> On 6/16/22 6:50 PM, Bodong Wang wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > BugLink: https://launchpad.net/bugs/1979009 > > When switching zones or network namespaces without doing a ct clear in > between, it is now leaking a reference to the old ct entry. That's > because tcf_ct_skb_nfct_cached() returns false and > tcf_ct_flow_table_lookup() may simply overwrite it. > > The fix is to, as the ct entry is not reusable, free it already at > tcf_ct_skb_nfct_cached(). > > Reported-by: Florian Westphal <fw@strlen.de> > Fixes: 2f131de361f6 ("net/sched: act_ct: Fix flow table lookup after ct clear or switching zones") > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Paul Blakey <paulb@nvidia.com> > (backported from commit bcb74e132a76ce0502bb33d5b65533a4ed72d159) > [paul: use ct_put rather than conntrack_put] > Signed-off-by: Bodong Wang <bodong@nvidia.com> > --- > net/sched/act_ct.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index ed310be..0f44608 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -598,22 +598,25 @@ static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb, > if (!ct) > return false; > if (!net_eq(net, read_pnet(&ct->ct_net))) > - return false; > + goto drop_ct; > if (nf_ct_zone(ct)->id != zone_id) > - return false; > + goto drop_ct; > > /* Force conntrack entry direction. */ > if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) { > if (nf_ct_is_confirmed(ct)) > nf_ct_kill(ct); > > - nf_conntrack_put(&ct->ct_general); > - nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > - > - return false; > + goto drop_ct; > } > > return true; > + > +drop_ct: > + nf_ct_put(ct); > + nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > + > + return false; > } > > /* Trim the skb to the length specified by the IP/IPv6 header,
On 6/16/22 16:50, Bodong Wang wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > BugLink: https://launchpad.net/bugs/1979009 > > When switching zones or network namespaces without doing a ct clear in > between, it is now leaking a reference to the old ct entry. That's > because tcf_ct_skb_nfct_cached() returns false and > tcf_ct_flow_table_lookup() may simply overwrite it. > > The fix is to, as the ct entry is not reusable, free it already at > tcf_ct_skb_nfct_cached(). > > Reported-by: Florian Westphal <fw@strlen.de> > Fixes: 2f131de361f6 ("net/sched: act_ct: Fix flow table lookup after ct clear or switching zones") > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Paul Blakey <paulb@nvidia.com> > (backported from commit bcb74e132a76ce0502bb33d5b65533a4ed72d159) > [paul: use ct_put rather than conntrack_put] > Signed-off-by: Bodong Wang <bodong@nvidia.com> > --- > net/sched/act_ct.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index ed310be..0f44608 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -598,22 +598,25 @@ static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb, > if (!ct) > return false; > if (!net_eq(net, read_pnet(&ct->ct_net))) > - return false; > + goto drop_ct; > if (nf_ct_zone(ct)->id != zone_id) > - return false; > + goto drop_ct; > > /* Force conntrack entry direction. */ > if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) { > if (nf_ct_is_confirmed(ct)) > nf_ct_kill(ct); > > - nf_conntrack_put(&ct->ct_general); > - nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > - > - return false; > + goto drop_ct; > } > > return true; > + > +drop_ct: > + nf_ct_put(ct); > + nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > + > + return false; > } > > /* Trim the skb to the length specified by the IP/IPv6 header, Acked-by: Tim Gardner <tim.gardner@canonical.com>
Applied to f:bluefield/master-next. Thanks, Zack On 6/16/22 6:50 PM, Bodong Wang wrote: > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > > BugLink: https://launchpad.net/bugs/1979009 > > When switching zones or network namespaces without doing a ct clear in > between, it is now leaking a reference to the old ct entry. That's > because tcf_ct_skb_nfct_cached() returns false and > tcf_ct_flow_table_lookup() may simply overwrite it. > > The fix is to, as the ct entry is not reusable, free it already at > tcf_ct_skb_nfct_cached(). > > Reported-by: Florian Westphal <fw@strlen.de> > Fixes: 2f131de361f6 ("net/sched: act_ct: Fix flow table lookup after ct clear or switching zones") > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Paul Blakey <paulb@nvidia.com> > (backported from commit bcb74e132a76ce0502bb33d5b65533a4ed72d159) > [paul: use ct_put rather than conntrack_put] > Signed-off-by: Bodong Wang <bodong@nvidia.com> > --- > net/sched/act_ct.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index ed310be..0f44608 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -598,22 +598,25 @@ static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb, > if (!ct) > return false; > if (!net_eq(net, read_pnet(&ct->ct_net))) > - return false; > + goto drop_ct; > if (nf_ct_zone(ct)->id != zone_id) > - return false; > + goto drop_ct; > > /* Force conntrack entry direction. */ > if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) { > if (nf_ct_is_confirmed(ct)) > nf_ct_kill(ct); > > - nf_conntrack_put(&ct->ct_general); > - nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > - > - return false; > + goto drop_ct; > } > > return true; > + > +drop_ct: > + nf_ct_put(ct); > + nf_ct_set(skb, NULL, IP_CT_UNTRACKED); > + > + return false; > } > > /* Trim the skb to the length specified by the IP/IPv6 header,
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index ed310be..0f44608 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -598,22 +598,25 @@ static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb, if (!ct) return false; if (!net_eq(net, read_pnet(&ct->ct_net))) - return false; + goto drop_ct; if (nf_ct_zone(ct)->id != zone_id) - return false; + goto drop_ct; /* Force conntrack entry direction. */ if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) { if (nf_ct_is_confirmed(ct)) nf_ct_kill(ct); - nf_conntrack_put(&ct->ct_general); - nf_ct_set(skb, NULL, IP_CT_UNTRACKED); - - return false; + goto drop_ct; } return true; + +drop_ct: + nf_ct_put(ct); + nf_ct_set(skb, NULL, IP_CT_UNTRACKED); + + return false; } /* Trim the skb to the length specified by the IP/IPv6 header,