diff mbox

[dpdk-dev,v2,3/3] malloc: fix potential dereferencing of NULL pointer

Message ID d6d551f5a856d1637a03d7689634d8d434f97c91.1524651111.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anatoly Burakov April 25, 2018, 10:15 a.m. UTC 
  Previous code checked for both first/last elements being NULL,
but if they weren't, the expectation was that they're both
non-NULL, which will be the case under normal conditions, but
may not be the case due to heap structure corruption.

Coverity issue: 272566

Fixes: bb372060dad4 ("malloc: make heap a doubly-linked list")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/malloc_elem.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Van Haaren, Harry April 27, 2018, 3:57 p.m. UTC | #1 
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Wednesday, April 25, 2018 11:16 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH v2 3/3] malloc: fix potential dereferencing of NULL
> pointer
> 
> Previous code checked for both first/last elements being NULL,
> but if they weren't, the expectation was that they're both
> non-NULL, which will be the case under normal conditions, but
> may not be the case due to heap structure corruption.
> 
> Coverity issue: 272566
> 
> Fixes: bb372060dad4 ("malloc: make heap a doubly-linked list")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Had to do a double-take there - that's a novel way of checking
pointers - but it actually makes sense here :)

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Anatoly Burakov April 27, 2018, 4:02 p.m. UTC | #2 
On 27-Apr-18 4:57 PM, Van Haaren, Harry wrote:
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
>> Sent: Wednesday, April 25, 2018 11:16 AM
>> To: dev@dpdk.org
>> Cc: thomas@monjalon.net; Burakov, Anatoly <anatoly.burakov@intel.com>
>> Subject: [dpdk-dev] [PATCH v2 3/3] malloc: fix potential dereferencing of NULL
>> pointer
>>
>> Previous code checked for both first/last elements being NULL,
>> but if they weren't, the expectation was that they're both
>> non-NULL, which will be the case under normal conditions, but
>> may not be the case due to heap structure corruption.
>>
>> Coverity issue: 272566
>>
>> Fixes: bb372060dad4 ("malloc: make heap a doubly-linked list")
>> Cc: anatoly.burakov@intel.com
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Had to do a double-take there - that's a novel way of checking
> pointers - but it actually makes sense here :)
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> 

It's basically a logical XOR :)

Thanks for reviewing!
diff mbox

Patch

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index ee79dcd..af81961 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -49,6 +49,12 @@  malloc_elem_insert(struct malloc_elem *elem)
 	struct malloc_elem *prev_elem, *next_elem;
 	struct malloc_heap *heap = elem->heap;
 
+	/* first and last elements must be both NULL or both non-NULL */
+	if ((heap->first == NULL) != (heap->last == NULL)) {
+		RTE_LOG(ERR, EAL, "Heap is probably corrupt\n");
+		return;
+	}
+
 	if (heap->first == NULL && heap->last == NULL) {
 		/* if empty heap */
 		heap->first = elem;