From 77ac5e40c44eb78333fbc38482d61fc2af7dda0a Mon Sep 17 00:00:00 2001 From: Louis Peens Date: Fri, 2 Jul 2021 11:21:38 +0200 Subject: [PATCH 1/2] net/sched: act_ct: remove and free nf_table callbacks 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 Signed-off-by: Yinjun Zhang Signed-off-by: Simon Horman Signed-off-by: David S. Miller --- 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 a62f404d0e165..1b4b3514c94f2 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); From 7cc93d888df764a13f196e3d4aef38869f7dd217 Mon Sep 17 00:00:00 2001 From: Louis Peens Date: Fri, 2 Jul 2021 11:21:39 +0200 Subject: [PATCH 2/2] nfp: flower-ct: remove callback delete deadlock The current location of the callback delete can lead to a race condition where deleting the callback requires a write_lock on the nf_table, but at the same time another thread from netfilter could have taken a read lock on the table before trying to offload. Since the driver is taking a rtnl_lock this can lead into a deadlock situation, where the netfilter offload will wait for the cls_flower rtnl_lock to be released, but this cannot happen since this is waiting for the nf_table read_lock to be released before it can delete the callback. Solve this by completely removing the nf_flow_table_offload_del_cb call, as this will now be cleaned up by act_ct itself when cleaning up the specific nf_table. Fixes: 62268e78145f ("nfp: flower-ct: add nft callback stubs") Signed-off-by: Louis Peens Signed-off-by: Yinjun Zhang Signed-off-by: Simon Horman Signed-off-by: David S. Miller --- .../net/ethernet/netronome/nfp/flower/conntrack.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c index 273d529d43c20..128020b1573e6 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c +++ b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c @@ -1141,20 +1141,7 @@ int nfp_fl_ct_del_flow(struct nfp_fl_ct_map_entry *ct_map_ent) nfp_fl_ct_clean_flow_entry(ct_entry); kfree(ct_map_ent); - /* If this is the last pre_ct_rule it means that it is - * very likely that the nft table will be cleaned up next, - * as this happens on the removal of the last act_ct flow. - * However we cannot deregister the callback on the removal - * of the last nft flow as this runs into a deadlock situation. - * So deregister the callback on removal of the last pre_ct flow - * and remove any remaining nft flow entries. We also cannot - * save this state and delete the callback later since the - * nft table would already have been freed at that time. - */ if (!zt->pre_ct_count) { - nf_flow_table_offload_del_cb(zt->nft, - nfp_fl_ct_handle_nft_flow, - zt); zt->nft = NULL; nfp_fl_ct_clean_nft_entries(zt); }