On Tue, Oct 02, 2018 at 12:50:19PM -0700, Cong Wang wrote: > In commit ec3ed293e766 ("net_sched: change tcf_del_walker() to take > idrinfo->lock") > we move fl_hw_destroy_tmplt() to a workqueue to avoid blocking > with the spinlock held. Unfortunately, this causes a lot of > troubles here: > > 1. tcf_chain_destroy() could be called right after we queue the work > but before the work runs. This is a use-after-free. > > 2. The chain refcnt is already 0, we can't even just hold it again. > We can check refcnt==1 but it is ugly. > > 3. The chain with refcnt 0 is still visible in its block, which means > it could be still found and used! > > 4. The block has a refcnt too, we can't hold it without introducing a > proper API either. > > We can make it working but the end result is ugly. Instead of wasting > time on reviewing it, let's just convert the troubling spinlock to > a mutex, which allows us to use non-atomic allocations too. > > Fixes: ec3ed293e766 ("net_sched: change tcf_del_walker() to take > idrinfo->lock") > Reported-by: Ido Schimmel <ido...@idosch.org> > Cc: Jamal Hadi Salim <j...@mojatatu.com> > Cc: Vlad Buslov <vla...@mellanox.com> > Cc: Jiri Pirko <j...@mellanox.com> > Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
Tested-by: Ido Schimmel <ido...@mellanox.com> Thanks a lot!