From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0677CA0573; Thu, 5 Mar 2020 10:20:01 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DD43A1BFE6; Thu, 5 Mar 2020 10:20:00 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 257F81BFCE for ; Thu, 5 Mar 2020 10:19:58 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Mar 2020 01:19:58 -0800 X-IronPort-AV: E=Sophos;i="5.70,517,1574150400"; d="scan'208";a="229634957" Received: from bricha3-mobl.ger.corp.intel.com ([10.251.89.129]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 05 Mar 2020 01:19:55 -0800 Date: Thu, 5 Mar 2020 09:19:52 +0000 From: Bruce Richardson To: ZY Qiu Cc: Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , dev@dpdk.org, ZY Qiu Message-ID: <20200305091952.GA289@bricha3-MOBL.ger.corp.intel.com> References: <20200304140543.31612-1-tgw_team@tencent.com> <20200304173349.26459-1-tgw_team@tencent.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200304173349.26459-1-tgw_team@tencent.com> User-Agent: Mutt/1.12.1 (2019-06-15) Subject: Re: [dpdk-dev] [PATCH v2] rte_ethdev: safer memory access by calling Rx callback X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Mar 05, 2020 at 01:33:49AM +0800, ZY Qiu wrote: > When compiling with -O0, > the compiler does not optimize two memory accesses into one. > Leads to accessing a null pointer when queue post Rx burst callback > removal while traffic is running. > See rte_eth_tx_burst function. > > Signed-off-by: ZY Qiu > --- > lib/librte_ethdev/rte_ethdev.h | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > index d1a593ad1..35eb580ff 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -4388,10 +4388,8 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id, > rx_pkts, nb_pkts); > > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > - if (unlikely(dev->post_rx_burst_cbs[queue_id] != NULL)) { > - struct rte_eth_rxtx_callback *cb = > - dev->post_rx_burst_cbs[queue_id]; > - > + struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id]; > + if (unlikely(cb != NULL)) { > do { > nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx, > nb_pkts, cb->param); > -- > 2.17.1 While I don't have an issue with this fix, can you explain as to why this is a problem that needs to be fixed? Normally TOCTOU issues are flagged and fixed for external resources e.g. files, that can be modified between check and use, but this is just referencing internal data in the program itself, so I'm wondering what the risk is? From a security viewpoint if an attacker can modify the function pointers in our code, is it not already "game over" for keeping the running program safe? /Bruce