[dpdk-dev] [PATCH 6/6] eal/linux: avoid out of bound access

[David Marchand <david.marchand@6wind.com>]

Using IBM advance toolchain on Ubuntu 14.04 (package 8.0-3), gcc is complaining
about out of bound accesses.

  CC eal_hugepage_info.o
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c:
In function ‘eal_hugepage_info_init’:
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c:350:35:
error: array subscript is above array bounds [-Werror=array-bounds]
      internal_config.hugepage_info[j].hugepage_sz)
                                   ^
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c:350:35:
error: array subscript is above array bounds [-Werror=array-bounds]
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c:349:37:
error: array subscript is above array bounds [-Werror=array-bounds]
    if (internal_config.hugepage_info[j-1].hugepage_sz <
                                     ^
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c:350:35:
error: array subscript is above array bounds [-Werror=array-bounds]
      internal_config.hugepage_info[j].hugepage_sz)

Looking at the code, these warnings are invalid from my pov and they disappeared
when upgrading the toolchain to new version (8.0-4).

However, the code was buggy (sorting code is wrong), so fix this by using qsort and adding a
check on num_sizes to avoid potential out of bound accesses.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c |   31 ++++++++++-------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index df60f6e..2f96164 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -184,15 +184,6 @@ get_hugepage_dir(uint64_t hugepage_sz)
 	return retval;
 }
 
-static inline void
-swap_hpi(struct hugepage_info *a, struct hugepage_info *b)
-{
-	char buf[sizeof(*a)];
-	memcpy(buf, a, sizeof(buf));
-	memcpy(a, b, sizeof(buf));
-	memcpy(b, buf, sizeof(buf));
-}
-
 /*
  * Clear the hugepage directory of whatever hugepage files
  * there are. Checks if the file is locked (i.e.
@@ -263,6 +254,15 @@ error:
 	return -1;
 }
 
+static int
+compare_hpi(const void *a, const void *b)
+{
+	const struct hugepage_info *hpi_a = a;
+	const struct hugepage_info *hpi_b = b;
+
+	return hpi_b->hugepage_sz - hpi_a->hugepage_sz;
+}
+
 /*
  * when we initialize the hugepage info, everything goes
  * to socket 0 by default. it will later get sorted by memory
@@ -289,6 +289,9 @@ eal_hugepage_info_init(void)
 			    dirent_start_len) != 0)
 			continue;
 
+		if (num_sizes >= MAX_HUGEPAGE_SIZES)
+			break;
+
 		hpi = &internal_config.hugepage_info[num_sizes];
 		hpi->hugepage_sz =
 			rte_str_to_size(&dirent->d_name[dirent_start_len]);
@@ -343,14 +346,8 @@ eal_hugepage_info_init(void)
 	internal_config.num_hugepage_sizes = num_sizes;
 
 	/* sort the page directory entries by size, largest to smallest */
-	for (i = 0; i < num_sizes; i++) {
-		unsigned j;
-		for (j = i+1; j < num_sizes; j++)
-			if (internal_config.hugepage_info[j-1].hugepage_sz <
-			     internal_config.hugepage_info[j].hugepage_sz)
-				swap_hpi(&internal_config.hugepage_info[j-1],
-					 &internal_config.hugepage_info[j]);
-	}
+	qsort(&internal_config.hugepage_info[0], num_sizes,
+	      sizeof(internal_config.hugepage_info[0]), compare_hpi);
 
 	/* now we have all info, check we have at least one valid size */
 	for (i = 0; i < num_sizes; i++)
-- 
1.7.10.4