From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ganesha.gnumonks.org (ganesha.gnumonks.org [IPv6:2001:780:45:1d:225:90ff:fe52:c662]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 546A1B3 for ; Mon, 11 Dec 2023 01:30:25 -0800 (PST) Received: from [78.30.43.141] (port=42842 helo=gnumonks.org) by ganesha.gnumonks.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rCccD-00GnZc-SG; Mon, 11 Dec 2023 10:30:19 +0100 Date: Mon, 11 Dec 2023 10:30:16 +0100 From: Pablo Neira Ayuso To: Jakub Kicinski Cc: jhs@mojatatu.com, Vlad Buslov , davem@davemloft.net, pabeni@redhat.com, edumazet@google.com, netdev@vger.kernel.org, louis.peens@corigine.com, yinjun.zhang@corigine.com, simon.horman@corigine.com, jiri@resnulli.us, xiyou.wangcong@gmail.com, Paul Blakey Subject: Re: [PATCH net] net/sched: act_ct: Take per-cb reference to tcf_ct_flow_table Message-ID: References: <20231205172554.3570602-1-vladbu@nvidia.com> <20231208154035.7cbec2f7@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20231208154035.7cbec2f7@kernel.org> X-Spam-Score: -1.9 (-) On Fri, Dec 08, 2023 at 03:40:35PM -0800, Jakub Kicinski wrote: > On Tue, 5 Dec 2023 18:25:54 +0100 Vlad Buslov wrote: > > The referenced change added custom cleanup code to act_ct to delete any > > callbacks registered on the parent block when deleting the > > tcf_ct_flow_table instance. However, the underlying issue is that the > > drivers don't obtain the reference to the tcf_ct_flow_table instance when > > registering callbacks which means that not only driver callbacks may still > > be on the table when deleting it but also that the driver can still have > > pointers to its internal nf_flowtable and can use it concurrently which > > results either warning in netfilter[0] or use-after-free. > > > > Fix the issue by taking a reference to the underlying struct > > tcf_ct_flow_table instance when registering the callback and release the > > reference when unregistering. Expose new API required for such reference > > counting by adding two new callbacks to nf_flowtable_type and implementing > > them for act_ct flowtable_ct type. This fixes the issue by extending the > > lifetime of nf_flowtable until all users have unregistered. > > Some acks would be good here - Pablo, Jamal? Acked-by: Pablo Neira Ayuso I'd prefer driver does not access nf_flowtable directly, I already mentioned this in the past.