hash: fix dereference before null check

Message ID 166b4a84232777b5ff3dd5d659140dab104e3f77.1603782481.git.wangyunjian@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series hash: fix dereference before null check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Yunjian Wang Oct. 27, 2020, 12:51 p.m. UTC
From: Yunjian Wang <wangyunjian@huawei.com>

Coverity flags that 'h' variable is used before
it's checked for NULL. This patch fixes this issue.

Coverity issue: 363625
Fixes: 769b2de7fb52 ("hash: implement RCU resources reclamation")
Cc: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Comments

Dharmik Thakkar Oct. 27, 2020, 2:59 p.m. UTC | #1
Hi Yunjian,

Thank you for fixing the issue!

> On Oct 27, 2020, at 7:51 AM, wangyunjian <wangyunjian@huawei.com> wrote:
>
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Coverity flags that 'h' variable is used before
> it's checked for NULL. This patch fixes this issue.
>
> Coverity issue: 363625
> Fixes: 769b2de7fb52 ("hash: implement RCU resources reclamation")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
> lib/librte_hash/rte_cuckoo_hash.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
> index 7514e33aa9..1191dfd81a 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -1515,15 +1515,16 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
> struct rte_rcu_qsbr_dq_parameters params = {0};
> char rcu_dq_name[RTE_RCU_QSBR_DQ_NAMESIZE];
> struct rte_hash_rcu_config *hash_rcu_cfg = NULL;
> -const uint32_t total_entries = h->use_local_cache ?
> -h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
> -: h->entries + 1;
>
> if (h == NULL || cfg == NULL || cfg->v == NULL) {
> rte_errno = EINVAL;
> return 1;
> }
>
> +const uint32_t total_entries = h->use_local_cache ?
> +h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
> +: h->entries + 1;
> +
> if (h->hash_rcu_cfg) {
> rte_errno = EEXIST;
> return 1;
> —
> 2.23.0
>


Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Dharmik Thakkar Oct. 27, 2020, 3:08 p.m. UTC | #2
Apologies for the disclaimer!

> On Oct 27, 2020, at 9:59 AM, Dharmik Thakkar <Dharmik.Thakkar@arm.com> wrote:
> 
> Hi Yunjian,
> 
> Thank you for fixing the issue!
> 
>> On Oct 27, 2020, at 7:51 AM, wangyunjian <wangyunjian@huawei.com> wrote:
>> 
>> From: Yunjian Wang <wangyunjian@huawei.com>
>> 
>> Coverity flags that 'h' variable is used before
>> it's checked for NULL. This patch fixes this issue.
>> 
>> Coverity issue: 363625
>> Fixes: 769b2de7fb52 ("hash: implement RCU resources reclamation")
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
>> ---
>> lib/librte_hash/rte_cuckoo_hash.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
>> index 7514e33aa9..1191dfd81a 100644
>> --- a/lib/librte_hash/rte_cuckoo_hash.c
>> +++ b/lib/librte_hash/rte_cuckoo_hash.c
>> @@ -1515,15 +1515,16 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
>> struct rte_rcu_qsbr_dq_parameters params = {0};
>> char rcu_dq_name[RTE_RCU_QSBR_DQ_NAMESIZE];
>> struct rte_hash_rcu_config *hash_rcu_cfg = NULL;
>> -const uint32_t total_entries = h->use_local_cache ?
>> -h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
>> -: h->entries + 1;
>> 
>> if (h == NULL || cfg == NULL || cfg->v == NULL) {
>> rte_errno = EINVAL;
>> return 1;
>> }
>> 
>> +const uint32_t total_entries = h->use_local_cache ?
>> +h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
>> +: h->entries + 1;
>> +
>> if (h->hash_rcu_cfg) {
>> rte_errno = EEXIST;
>> return 1;
>> —
>> 2.23.0
>> 
> 
> 
> Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>
  
Wang, Yipeng1 Oct. 28, 2020, 4:25 p.m. UTC | #3
> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Tuesday, October 27, 2020 5:51 AM
> To: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> <sameh.gobriel@intel.com>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>;
> dharmik.thakkar@arm.com; jerry.lilijun@huawei.com; xudingke@huawei.com;
> Yunjian Wang <wangyunjian@huawei.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] hash: fix dereference before null check
> 
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Coverity flags that 'h' variable is used before it's checked for NULL. This patch
> fixes this issue.
> 
> Coverity issue: 363625
> Fixes: 769b2de7fb52 ("hash: implement RCU resources reclamation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
 [Wang, Yipeng] 
Thanks for the fix!

Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
  
David Marchand Oct. 29, 2020, 4:09 p.m. UTC | #4
On Tue, Oct 27, 2020 at 1:52 PM wangyunjian <wangyunjian@huawei.com> wrote:
>
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Coverity flags that 'h' variable is used before
> it's checked for NULL. This patch fixes this issue.
>
> Coverity issue: 363625
> Fixes: 769b2de7fb52 ("hash: implement RCU resources reclamation")

No need to copy stable, this feature is only in the current release.


> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>


Applied, thanks.
  

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 7514e33aa9..1191dfd81a 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -1515,15 +1515,16 @@  rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
 	struct rte_rcu_qsbr_dq_parameters params = {0};
 	char rcu_dq_name[RTE_RCU_QSBR_DQ_NAMESIZE];
 	struct rte_hash_rcu_config *hash_rcu_cfg = NULL;
-	const uint32_t total_entries = h->use_local_cache ?
-		h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
-							: h->entries + 1;
 
 	if (h == NULL || cfg == NULL || cfg->v == NULL) {
 		rte_errno = EINVAL;
 		return 1;
 	}
 
+	const uint32_t total_entries = h->use_local_cache ?
+		h->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1
+							: h->entries + 1;
+
 	if (h->hash_rcu_cfg) {
 		rte_errno = EEXIST;
 		return 1;