Coverity fixes for EAL

This patchset fixes a few Coverity fixes in EAL introduced by recent DPDK memory hotplug patchset.

Coverity issues fixed:
- 272600 - negative return not handled
- 272607 - error condition not handled

One of existing coverity issues is not fixed by this patchset:
- 272585 - memory leak

due to being independently discovered and addressed in a separate patch [1].

[1] http://dpdk.org/dev/patchwork/patch/38301/

Anatoly Burakov (2):
eal: fix potential negative return
eal: fix not checking unlock result

 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

2.7.4

Value returned by rte_socket_id_by_idx() may be negative, which would result in negative array index access.

Coverity issue: 272600

---
 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

index fb4b667..009f963 100644
@@ -402,9 +402,16 @@ hugepage_info_init(void)
 		/* we also don't want to do this for legacy init */
 		if (!internal_config.legacy_mem)
 			for (i = 0; i < rte_socket_count(); i++) {
-				int socket = rte_socket_id_by_idx(i);
-				unsigned int num_pages =
-						get_num_hugepages_on_node(
+				int socket;
+				unsigned int num_pages;
+
+				socket = rte_socket_id_by_idx(i);
+				if (socket < 0) {
+					RTE_LOG(ERR, EAL, "Invalid socket index: %u\n",
+							i);
+					continue;
+				}
+				num_pages = get_num_hugepages_on_node(
 							dirent->d_name, socket);
 				hpi->num_pages[socket] = num_pages;
 				total_pages += num_pages;

2.7.4

Coverity issue: 272607

---
 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index 009f963..01fee51 100644
@@ -304,8 +304,11 @@ clear_hugedir(const char * hugedir)
 		/* if lock succeeds, unlock and remove the file */
 		if (lck_result != -1) {
 			lck.l_type = F_UNLCK;
-			fcntl(fd, F_SETLK, &lck);
-			unlinkat(dir_fd, dirent->d_name, 0);
+			if (fcntl(fd, F_SETLK, &lck) < 0)
+				RTE_LOG(ERR, EAL, "Couldn't unlock %s: %s\n",
+					dirent->d_name, strerror(errno));
+			else
+				unlinkat(dir_fd, dirent->d_name, 0);
 		}
 		close (fd);
 		dirent = readdir(dir);

2.7.4

I'm not sure if we should. In any case, lock will be dropped by close(), so the proper fix would've been just remove the fcntl() call altogether.

I'll respin.

Thanks,
Anatoly

This patchset fixes a few Coverity fixes in EAL introduced by recent DPDK memory hotplug patchset.

Coverity issues fixed:
- 272607 - error condition not handled

Coverity issues not fixed:
- 272600 - negative return not handled
- Proper usage of API guarantees no negative return

One of existing coverity issues is not fixed by this patchset:
- 272585 - memory leak

due to being independently discovered and addressed in a separate patch [1].

[1] http://dpdk.org/dev/patchwork/patch/38301/

v2:
- Dropped fix for 272600 and marked it as false positive

Anatoly Burakov (1):
eal: remove call to unlock

 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

2.7.4

Coverity was complaining about not checking result of call to fcntl() for unlocking the file. Disregarding the fact that error value returned from fcntl() unlock call is highly unlikely in the first place, we are subsequently calling close() on that same fd, which will drop the lock, which makes call to fcntl() unnecessary.

Fix this by removing a call to fcntl() altogether.

Coverity issue: 272607

---

Notes:
v2:
- Removed call to fcntl() instead of handling return value

 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index db5aabd..485a89e 100644
@@ -304,11 +304,8 @@ clear_hugedir(const char * hugedir)
 		lck_result = fcntl(fd, F_SETLK, &lck);
 
 		/* if lock succeeds, unlock and remove the file */
-		if (lck_result != -1) {
-			lck.l_type = F_UNLCK;
-			fcntl(fd, F_SETLK, &lck);
+		if (lck_result != -1)
 			unlinkat(dir_fd, dirent->d_name, 0);
-		}
 		close (fd);
 		dirent = readdir(dir);
 	}

2.7.4

25/04/2018 12:08, Anatoly Burakov:

Applied, thanks