From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [Patch net-next] net_sched: convert idrinfo->lock from spinlock to a mutex Date: Fri, 05 Oct 2018 00:38:15 -0700 (PDT) Message-ID: <20181005.003815.1366909058469067585.davem@davemloft.net> References: <20181002195019.13522-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, jiri@resnulli.us, jhs@mojatatu.com, vladbu@mellanox.com, idosch@idosch.org, jiri@mellanox.com To: xiyou.wangcong@gmail.com Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:35756 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727830AbeJEOfo (ORCPT ); Fri, 5 Oct 2018 10:35:44 -0400 In-Reply-To: <20181002195019.13522-1-xiyou.wangcong@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: From: Cong Wang Date: Tue, 2 Oct 2018 12:50:19 -0700 > 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 > Cc: Jamal Hadi Salim > Cc: Vlad Buslov > Cc: Jiri Pirko > Signed-off-by: Cong Wang Applied.