| Message ID | 1625666548-28831-1-git-send-email-bodong@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,F:linux-bluefield,v2] net/sched: act_ct: remove and free nf_table callbacks | expand |
On 07.07.21 16:02, Bodong Wang wrote: > From: Louis Peens <louis.peens@corigine.com> > > BugLink: https://launchpad.net/bugs/1934822 > > When cleaning up the nf_table in tcf_ct_flow_table_cleanup_work > there is no guarantee that the callback list, added to by > nf_flow_table_offload_add_cb, is empty. This means that it is > possible that the flow_block_cb memory allocated will be lost. > > Fix this by iterating the list and freeing the flow_block_cb entries > before freeing the nf_table entry (via freeing ct_ft). > > Fixes: 978703f42549 ("netfilter: flowtable: Add API for registering to flow table events") > Signed-off-by: Louis Peens <louis.peens@corigine.com> > Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com> > Signed-off-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry-picked from commit 77ac5e40c44eb78333fbc38482d61fc2af7dda0a linux-next) > https://patchwork.kernel.org/project/netdevbpf/patch/20210702092139.25662-2-simon.horman@corigine.com/ > Signed-off-by: Bodong Wang <bodong@nvidia.com> > Acked-by: Tim Gardner <tim.gardner@canonical.com> Same comment here about carrying forward the ACK. No need for a re-submission. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Thanks > --- > net/sched/act_ct.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index 43c5b3f..99b9f1f 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -322,11 +322,22 @@ static int tcf_ct_flow_table_get(struct tcf_ct_params *params) > > static void tcf_ct_flow_table_cleanup_work(struct work_struct *work) > { > + struct flow_block_cb *block_cb, *tmp_cb; > struct tcf_ct_flow_table *ct_ft; > + struct flow_block *block; > > ct_ft = container_of(to_rcu_work(work), struct tcf_ct_flow_table, > rwork); > nf_flow_table_free(&ct_ft->nf_ft); > + > + /* Remove any remaining callbacks before cleanup */ > + block = &ct_ft->nf_ft.flow_block; > + down_write(&ct_ft->nf_ft.flow_block_lock); > + list_for_each_entry_safe(block_cb, tmp_cb, &block->cb_list, list) { > + list_del(&block_cb->list); > + flow_block_cb_free(block_cb); > + } > + up_write(&ct_ft->nf_ft.flow_block_lock); > kfree(ct_ft); > > module_put(THIS_MODULE); >
Acked-by: Tim Gardner <tim.gardner@canonical.com> On 7/7/21 8:02 AM, Bodong Wang wrote: > From: Louis Peens <louis.peens@corigine.com> > > BugLink: https://launchpad.net/bugs/1934822 > > When cleaning up the nf_table in tcf_ct_flow_table_cleanup_work > there is no guarantee that the callback list, added to by > nf_flow_table_offload_add_cb, is empty. This means that it is > possible that the flow_block_cb memory allocated will be lost. > > Fix this by iterating the list and freeing the flow_block_cb entries > before freeing the nf_table entry (via freeing ct_ft). > > Fixes: 978703f42549 ("netfilter: flowtable: Add API for registering to flow table events") > Signed-off-by: Louis Peens <louis.peens@corigine.com> > Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com> > Signed-off-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry-picked from commit 77ac5e40c44eb78333fbc38482d61fc2af7dda0a linux-next) > https://patchwork.kernel.org/project/netdevbpf/patch/20210702092139.25662-2-simon.horman@corigine.com/ > Signed-off-by: Bodong Wang <bodong@nvidia.com> > Acked-by: Tim Gardner <tim.gardner@canonical.com> > --- > net/sched/act_ct.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index 43c5b3f..99b9f1f 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -322,11 +322,22 @@ static int tcf_ct_flow_table_get(struct tcf_ct_params *params) > > static void tcf_ct_flow_table_cleanup_work(struct work_struct *work) > { > + struct flow_block_cb *block_cb, *tmp_cb; > struct tcf_ct_flow_table *ct_ft; > + struct flow_block *block; > > ct_ft = container_of(to_rcu_work(work), struct tcf_ct_flow_table, > rwork); > nf_flow_table_free(&ct_ft->nf_ft); > + > + /* Remove any remaining callbacks before cleanup */ > + block = &ct_ft->nf_ft.flow_block; > + down_write(&ct_ft->nf_ft.flow_block_lock); > + list_for_each_entry_safe(block_cb, tmp_cb, &block->cb_list, list) { > + list_del(&block_cb->list); > + flow_block_cb_free(block_cb); > + } > + up_write(&ct_ft->nf_ft.flow_block_lock); > kfree(ct_ft); > > module_put(THIS_MODULE); >
On 07.07.21 16:02, Bodong Wang wrote: > From: Louis Peens <louis.peens@corigine.com> > > BugLink: https://launchpad.net/bugs/1934822 > > When cleaning up the nf_table in tcf_ct_flow_table_cleanup_work > there is no guarantee that the callback list, added to by > nf_flow_table_offload_add_cb, is empty. This means that it is > possible that the flow_block_cb memory allocated will be lost. > > Fix this by iterating the list and freeing the flow_block_cb entries > before freeing the nf_table entry (via freeing ct_ft). > > Fixes: 978703f42549 ("netfilter: flowtable: Add API for registering to flow table events") > Signed-off-by: Louis Peens <louis.peens@corigine.com> > Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com> > Signed-off-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry-picked from commit 77ac5e40c44eb78333fbc38482d61fc2af7dda0a linux-next) > https://patchwork.kernel.org/project/netdevbpf/patch/20210702092139.25662-2-simon.horman@corigine.com/ > Signed-off-by: Bodong Wang <bodong@nvidia.com> > Acked-by: Tim Gardner <tim.gardner@canonical.com> > --- Applied to focal:linux-bluefield/master-next. Thanks. -Stefan > net/sched/act_ct.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index 43c5b3f..99b9f1f 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -322,11 +322,22 @@ static int tcf_ct_flow_table_get(struct tcf_ct_params *params) > > static void tcf_ct_flow_table_cleanup_work(struct work_struct *work) > { > + struct flow_block_cb *block_cb, *tmp_cb; > struct tcf_ct_flow_table *ct_ft; > + struct flow_block *block; > > ct_ft = container_of(to_rcu_work(work), struct tcf_ct_flow_table, > rwork); > nf_flow_table_free(&ct_ft->nf_ft); > + > + /* Remove any remaining callbacks before cleanup */ > + block = &ct_ft->nf_ft.flow_block; > + down_write(&ct_ft->nf_ft.flow_block_lock); > + list_for_each_entry_safe(block_cb, tmp_cb, &block->cb_list, list) { > + list_del(&block_cb->list); > + flow_block_cb_free(block_cb); > + } > + up_write(&ct_ft->nf_ft.flow_block_lock); > kfree(ct_ft); > > module_put(THIS_MODULE); >
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index 43c5b3f..99b9f1f 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -322,11 +322,22 @@ static int tcf_ct_flow_table_get(struct tcf_ct_params *params) static void tcf_ct_flow_table_cleanup_work(struct work_struct *work) { + struct flow_block_cb *block_cb, *tmp_cb; struct tcf_ct_flow_table *ct_ft; + struct flow_block *block; ct_ft = container_of(to_rcu_work(work), struct tcf_ct_flow_table, rwork); nf_flow_table_free(&ct_ft->nf_ft); + + /* Remove any remaining callbacks before cleanup */ + block = &ct_ft->nf_ft.flow_block; + down_write(&ct_ft->nf_ft.flow_block_lock); + list_for_each_entry_safe(block_cb, tmp_cb, &block->cb_list, list) { + list_del(&block_cb->list); + flow_block_cb_free(block_cb); + } + up_write(&ct_ft->nf_ft.flow_block_lock); kfree(ct_ft); module_put(THIS_MODULE);