[dpdk-dev] eal/linux: fix undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)

Message ID 1461776764-108197-1-git-send-email-danielx.t.mrzyglod@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Daniel Mrzyglod April 27, 2016, 5:06 p.m. UTC
  Fix issue reported by clang scan-build

there is a chance that nr_hugepages will be 0 if conditions for loop
for (i = 0; i < (int) internal_config.num_hugepage_sizes; i++)
will be unmeet.

Fixes: b6a468ad41d5 ("memory: add --socket-mem option")

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memory.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Sergio Gonzalez Monroy June 8, 2016, 12:42 p.m. UTC | #1
On 27/04/2016 18:06, Daniel Mrzyglod wrote:
> Fix issue reported by clang scan-build
>
> there is a chance that nr_hugepages will be 0 if conditions for loop
> for (i = 0; i < (int) internal_config.num_hugepage_sizes; i++)
> will be unmeet.
>
> Fixes: b6a468ad41d5 ("memory: add --socket-mem option")
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 5b9132c..e94538e 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1114,6 +1114,8 @@ rte_eal_hugepage_init(void)
>   	 * processing done on these pages, shared memory will be created
>   	 * at a later stage.
>   	 */
> +	if (nr_hugepages == 0)
> +		goto fail;
>   	tmp_hp = malloc(nr_hugepages * sizeof(struct hugepage_file));
>   	if (tmp_hp == NULL)
>   		goto fail;

The behavior of malloc(0) is implementation-defined, but on Linux man 
page it says that returns NULL.
So strictly speaking, without the patch the outcome is the same cause 
malloc(0) will return NULL.

Now, I'd consider the patch not needed but it doesn't really harm either.
Anyone else has comments/thoughts about it?

Regarding the patch itself, I think the title and commit message need to 
be modify to reflect that the patch
goal is to handle nr_hugepages = 0 case without relying in malloc to 
return NULL.

Sergio
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 5b9132c..e94538e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1114,6 +1114,8 @@  rte_eal_hugepage_init(void)
 	 * processing done on these pages, shared memory will be created
 	 * at a later stage.
 	 */
+	if (nr_hugepages == 0)
+		goto fail;
 	tmp_hp = malloc(nr_hugepages * sizeof(struct hugepage_file));
 	if (tmp_hp == NULL)
 		goto fail;