[dpdk-dev,v2,3/3] malloc: fix potential dereferencing of NULL pointer
Checks
Commit Message
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
> 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>
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!
@@ -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;