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

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

[Daniel Mrzyglod]

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;
-- 
2.5.5

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

[Sergio Gonzalez Monroy]

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