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 9D2B0A04A2 for ; Mon, 18 May 2020 10:45:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 73AF01D51E; Mon, 18 May 2020 10:45:25 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 38E961D14B; Mon, 18 May 2020 10:45:21 +0200 (CEST) IronPort-SDR: q0ik2s+TJsje+AHM5NtYPOYmqBrwsDqfLdPal0vmbB37AJZ7Gl6zlGNtvcJtjumXXb37ThY8Sz +jVxPplOD7mg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2020 01:45:20 -0700 IronPort-SDR: KxFtvztDORsoIXcj/Ua+8TTTXWESdUnzoroTA6mz37tQJe0beQ/lr9FvpCT682raehISgELxmP VVCB1Qm2BZLw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,406,1583222400"; d="scan'208";a="465501314" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.187]) ([10.67.68.187]) by fmsmga005.fm.intel.com with ESMTP; 18 May 2020 01:45:18 -0700 To: Wei Zhao , dev@dpdk.org Cc: stable@dpdk.org, xiaolong.ye@intel.com References: <20200518074330.35840-1-wei.zhao1@intel.com> <20200518080051.36318-1-wei.zhao1@intel.com> From: Jeff Guo Message-ID: Date: Mon, 18 May 2020 16:45:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20200518080051.36318-1-wei.zhao1@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-stable] [PATCH v5] net/i40e: fix the core dump risk of wild pointer operation X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 5/18/2020 4:00 PM, Wei Zhao wrote: > In i40e PMD code of function i40e_res_pool_free(), if valid_entry > is freed by "rte_free(valid_entry);" in the code, then the following > code for pool update may still use the wild pointer "valid_entry" > for pool info update. It seems has the risk of core dump for > using wild pointer operation, we should avoid this risk. > > Cc: stable@dpdk.org > Fixes: 4861cde46116 ("i40e: new poll mode driver") > > Signed-off-by: Wei Zhao > > --- > > v2: > update commit log > > v3: > set free pointer to NULL > > v4: > change code style > > v5: > fix an issue in v4 > --- > drivers/net/i40e/i40e_ethdev.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c > index 749d85f54..00bb05179 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -4935,6 +4935,7 @@ i40e_res_pool_free(struct i40e_res_pool_info *pool, > { > struct pool_entry *entry, *next, *prev, *valid_entry = NULL; > uint32_t pool_offset; > + uint16_t len; > int insert; > > if (pool == NULL) { > @@ -4973,12 +4974,13 @@ i40e_res_pool_free(struct i40e_res_pool_info *pool, > } > > insert = 0; > + len = valid_entry->len; > /* Try to merge with next one*/ > if (next != NULL) { > /* Merge with next one */ > - if (valid_entry->base + valid_entry->len == next->base) { > + if (valid_entry->base + len == next->base) { > next->base = valid_entry->base; > - next->len += valid_entry->len; > + next->len += len; > rte_free(valid_entry); > valid_entry = next; > insert = 1; > @@ -4988,13 +4990,15 @@ i40e_res_pool_free(struct i40e_res_pool_info *pool, > if (prev != NULL) { > /* Merge with previous one */ > if (prev->base + prev->len == valid_entry->base) { > - prev->len += valid_entry->len; > + prev->len += len; > /* If it merge with next one, remove next node */ > if (insert == 1) { > LIST_REMOVE(valid_entry, next); > rte_free(valid_entry); > + valid_entry = NULL; > } else { > rte_free(valid_entry); > + valid_entry = NULL; > insert = 1; > } > } > @@ -5010,8 +5014,8 @@ i40e_res_pool_free(struct i40e_res_pool_info *pool, > LIST_INSERT_HEAD(&pool->free_list, valid_entry, next); > } > > - pool->num_free += valid_entry->len; > - pool->num_alloc -= valid_entry->len; > + pool->num_free += len; > + pool->num_alloc -= len; > > return 0; > } Reviewed-by: Jeff Guo