Hi John,
I do not know how I can test this fully with certainty (yes, I can set up a project for myself in Coverity in general) so I share it here too. I hope it helps.
Goal
Create a Coverity model that distinguishes between manual management (when allocator == NULL) and scoped management (when allocator is a valid pointer).
- When the allocator is NULL, treat the function like a standard malloc.
- When there is a scope, "sink" the pointer into that scope so Coverity understands the allocator will handle the cleanup.
Expected Results
Scenario: Manual
Allocator Passed: NULL
Coverity Behavior: Calls __coverity_alloc__. If no wmem_free(NULL, ptr) is found, a RESOURCE_LEAK is raised.
Scenario: Modern
Allocator Passed: Dissector
Coverity Behavior: pinfo->pool Calls wmem_alloc. Pointer is assigned to pool->storage_sink. No RESOURCE_LEAK raised.
Scenario: Epan Scope
Allocator Passed: wmem_epan_scope()
Coverity Behavior: Returns the singleton. Pointer is assigned to sink. No RESOURCE_LEAK raised.
Scenario: File Scope
Allocator Passed: wmem_file_scope()
Coverity Behavior: Returns the singleton. Pointer is assigned to sink. No RESOURCE_LEAK raised.
Examples
Scenario: Manual
Code Example: ptr = wmem_strdup(NULL, "test");
Coverity Result: RESOURCE_LEAK raised
Scenario: Manual
Code Example: ptr = wmem_strdup(NULL, "test");
Coverity Result: No issue
Scenario: Dissector Scope
Code Example: wmem_alloc(pinfo->pool, 10);
Coverity Result: No leak (escapes to pinfo->pool)
Scenario: EPAN/File Scope
Code Example: wmem_strdup(wmem_file_scope(), "data");
Coverity Result: No leak (escapes to singleton sink)
Regards,
Tamas
Hi John,
Your feedback is valuable and this case is complex indeed.
In case the allocator is clearly defined as NULL (and there are some cases such as tvb_memdup(NULL, ...) ) then we shall have wmem_free(NULL, ...) followed later. If wmem_free() is missing, that should be reported by Coverity as a resource leak and we shall not ignore it.
I am not a Coverity expert but I will try to understand more if there is a way to improve this specific resource leak scenario when allocator is clearly not NULL.
Thank you.
Regards,
Tamas
Hi Pascal,
Was there any consideration or discussion about a Coverity model file for these scenarios?
Regards,
Tamas
Hi Tamas,
If someone could do that, it would be welcome.
Complicating the matter somewhat is that the various wmem_() functions would leak memory if the wmem_allocator_t* passed in as the first argument is NULL. (This allocates with g_malloc, etc.) In practice pinfo->pool is never NULL (epan_dissect_init and epan_dissect_reset guarantee this), but if you examine many of the Coverity defect reports they take a branch assuming that the allocator is NULL. It might be necessary to do something like replace pinfo->pool with a macro that expands to simply pinfo->pool not on Coverity but to something that guarantees a non-NULL on Coverity, although that might not be sufficient.
Simply ignoring all the resource leaks would be a problem in the cases where those functions are called with NULL.
Cheers,
John
Wireshark-dev mailing list -- wireshark-dev@xxxxxxxxxxxxx
To unsubscribe send an email to wireshark-dev-leave@xxxxxxxxxxxxx
Attachment:
wmem_models.c
Description: Binary data