2023-04-14 12:35:15

by Duoming Zhou

Subject: [PATCH net] cxgb4: fix use after free bugs caused by circular dependency problem

The flower_stats_timer can schedule flower_stats_work and
flower_stats_work can also arm the flower_stats_timer. The
process is shown below:

----------- timer schedules work ------------
ch_flower_stats_cb() //timer handler
schedule_work(&adap->flower_stats_work);

----------- work arms timer ------------
ch_flower_stats_handler() //workqueue callback function
mod_timer(&adap->flower_stats_timer, ...);

When the cxgb4 device is detaching, the timer and workqueue
could still be rearmed. The process is shown below:

(cleanup routine) | (timer and workqueue routine)
remove_one() |
free_some_resources() | ch_flower_stats_cb() //timer
cxgb4_cleanup_tc_flower() | schedule_work()
del_timer_sync() |
| ch_flower_stats_handler() //workqueue
| mod_timer()
cancel_work_sync() |
kfree(adapter) //FREE | ch_flower_stats_cb() //timer
| adap->flower_stats_work //USE

This patch changes del_timer_sync() to timer_shutdown_sync(),
which could prevent rearming of the timer from the workqueue.

Fixes: e0f911c81e93 ("cxgb4: fetch stats for offloaded tc flower flows")
Signed-off-by: Duoming Zhou <[email protected]>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index dd9be229819..d3541159487 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -1135,7 +1135,7 @@ void cxgb4_cleanup_tc_flower(struct adapter *adap)
return;

if (adap->flower_stats_timer.function)
- del_timer_sync(&adap->flower_stats_timer);
+ timer_shutdown_sync(&adap->flower_stats_timer);
cancel_work_sync(&adap->flower_stats_work);
rhashtable_destroy(&adap->flower_tbl);
adap->tc_flower_initialized = false;
--
2.17.1

2023-04-15 01:27:17

by Jakub Kicinski

Subject: Re: [PATCH net] cxgb4: fix use after free bugs caused by circular dependency problem

On Fri, 14 Apr 2023 20:26:21 +0800 Duoming Zhou wrote:
> The flower_stats_timer can schedule flower_stats_work and
> flower_stats_work can also arm the flower_stats_timer. The
> process is shown below:

Please wait 24h for any comments, and if none arrive repost it with
_correctly_ populated CC list. Run get_maintiner on the patch.