Skip to content

Commit df45bf8

Browse files
jpirkodavem330
authored andcommitted
net: sched: fix use-after-free in tcf_block_put_ext
Since the block is freed with last chain being put, once we reach the end of iteration of list_for_each_entry_safe, the block may be already freed. I'm hitting this only by creating and deleting clsact: [ 202.171952] ================================================================== [ 202.180182] BUG: KASAN: use-after-free in tcf_block_put_ext+0x240/0x390 [ 202.187590] Read of size 8 at addr ffff880225539a80 by task tc/796 [ 202.194508] [ 202.196185] CPU: 0 PID: 796 Comm: tc Not tainted 4.15.0-rc2jiri+ #5 [ 202.203200] Hardware name: Mellanox Technologies Ltd. "MSN2100-CB2F"/"SA001017", BIOS 5.6.5 06/07/2016 [ 202.213613] Call Trace: [ 202.216369] dump_stack+0xda/0x169 [ 202.220192] ? dma_virt_map_sg+0x147/0x147 [ 202.224790] ? show_regs_print_info+0x54/0x54 [ 202.229691] ? tcf_chain_destroy+0x1dc/0x250 [ 202.234494] print_address_description+0x83/0x3d0 [ 202.239781] ? tcf_block_put_ext+0x240/0x390 [ 202.244575] kasan_report+0x1ba/0x460 [ 202.248707] ? tcf_block_put_ext+0x240/0x390 [ 202.253518] tcf_block_put_ext+0x240/0x390 [ 202.258117] ? tcf_chain_flush+0x290/0x290 [ 202.262708] ? qdisc_hash_del+0x82/0x1a0 [ 202.267111] ? qdisc_hash_add+0x50/0x50 [ 202.271411] ? __lock_is_held+0x5f/0x1a0 [ 202.275843] clsact_destroy+0x3d/0x80 [sch_ingress] [ 202.281323] qdisc_destroy+0xcb/0x240 [ 202.285445] qdisc_graft+0x216/0x7b0 [ 202.289497] tc_get_qdisc+0x260/0x560 Fix this by holding the block also by chain 0 and put chain 0 explicitly, out of the list_for_each_entry_safe loop at the very end of tcf_block_put_ext. Fixes: efbf789 ("net_sched: get rid of rcu_barrier() in tcf_block_put_ext()") Signed-off-by: Jiri Pirko <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent fc8b81a commit df45bf8

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

net/sched/cls_api.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,23 +342,24 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
342342
{
343343
struct tcf_chain *chain, *tmp;
344344

345-
/* Hold a refcnt for all chains, except 0, so that they don't disappear
345+
/* Hold a refcnt for all chains, so that they don't disappear
346346
* while we are iterating.
347347
*/
348348
list_for_each_entry(chain, &block->chain_list, list)
349-
if (chain->index)
350-
tcf_chain_hold(chain);
349+
tcf_chain_hold(chain);
351350

352351
list_for_each_entry(chain, &block->chain_list, list)
353352
tcf_chain_flush(chain);
354353

355354
tcf_block_offload_unbind(block, q, ei);
356355

357-
/* At this point, all the chains should have refcnt >= 1. Block will be
358-
* freed after all chains are gone.
359-
*/
356+
/* At this point, all the chains should have refcnt >= 1. */
360357
list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
361358
tcf_chain_put(chain);
359+
360+
/* Finally, put chain 0 and allow block to be freed. */
361+
chain = list_first_entry(&block->chain_list, struct tcf_chain, list);
362+
tcf_chain_put(chain);
362363
}
363364
EXPORT_SYMBOL(tcf_block_put_ext);
364365

0 commit comments

Comments
 (0)