[dpdk-dev] [PATCH] app/test lib/librte_pmd_af_packet: fix check for null & fix possible memory leak

[dpdk-dev] [PATCH] app/test lib/librte_pmd_af_packet: fix check for null & fix possible memory leak

[Daniel Mrzyglod]

app/test/test_sched.c:
-Fix several checking for NULL pointer
lib/librte_pmd_af_packet/rte_eth_af_packet.c:
-Not munmaped queue area
-Fix several checking for NULL pointer

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
 app/test/test_sched.c                        |  2 ++
 lib/librte_pmd_af_packet/rte_eth_af_packet.c | 49 ++++++++++++++++------------
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/app/test/test_sched.c b/app/test/test_sched.c
index c957d80..9b6e037 100644
--- a/app/test/test_sched.c
+++ b/app/test/test_sched.c
@@ -166,6 +166,7 @@ test_sched(void)
 	int err;
 
 	mp = create_mempool();
+	VERIFY(mp != NULL,"Error create mempool\n");
 
 	port_param.socket = 0;
 	port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8;
@@ -184,6 +185,7 @@ test_sched(void)
 
 	for (i = 0; i < 10; i++) {
 		in_mbufs[i] = rte_pktmbuf_alloc(mp);
+		VERIFY(in_mbufs[i] != NULL, "Bad packet allocation");
 		prepare_pkt(in_mbufs[i]);
 	}
 
diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
index d0fb3eb..d9f1373 100644
--- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c
+++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c
@@ -481,6 +481,11 @@ rte_pmd_init_internals(const char *name,
 	if (*internals == NULL)
 		goto error;
 
+	for (q = 0; q < nb_queues; q++) {
+		(*internals)->rx_queue[q].map = MAP_FAILED;
+		(*internals)->tx_queue[q].map = MAP_FAILED;
+	}
+
 	req = &((*internals)->req);
 
 	req->tp_block_size = blocksize;
@@ -496,13 +501,13 @@ rte_pmd_init_internals(const char *name,
 		RTE_LOG(ERR, PMD,
 			"%s: I/F name too long (%s)\n",
 			name, pair->value);
-		goto error;
+		goto error_free_queues;
 	}
 	if (ioctl(sockfd, SIOCGIFINDEX, &ifr) == -1) {
 		RTE_LOG(ERR, PMD,
 			"%s: ioctl failed (SIOCGIFINDEX)\n",
 		        name);
-		goto error;
+		goto error_free_queues;
 	}
 	(*internals)->if_index = ifr.ifr_ifindex;
 
@@ -510,7 +515,7 @@ rte_pmd_init_internals(const char *name,
 		RTE_LOG(ERR, PMD,
 			"%s: ioctl failed (SIOCGIFHWADDR)\n",
 		        name);
-		goto error;
+		goto error_free_queues;
 	}
 	memcpy(&(*internals)->eth_addr, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
 
@@ -544,7 +549,7 @@ rte_pmd_init_internals(const char *name,
 			RTE_LOG(ERR, PMD,
 				"%s: could not set PACKET_VERSION on AF_PACKET "
 				"socket for %s\n", name, pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 
 		discard = 1;
@@ -554,7 +559,7 @@ rte_pmd_init_internals(const char *name,
 			RTE_LOG(ERR, PMD,
 				"%s: could not set PACKET_LOSS on "
 			        "AF_PACKET socket for %s\n", name, pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 
 #if defined(PACKET_QDISC_BYPASS)
@@ -566,7 +571,7 @@ rte_pmd_init_internals(const char *name,
 				"%s: could not set PACKET_QDISC_BYPASS "
 			        "on AF_PACKET socket for %s\n", name,
 			        pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 #endif
 
@@ -575,7 +580,7 @@ rte_pmd_init_internals(const char *name,
 			RTE_LOG(ERR, PMD,
 				"%s: could not set PACKET_RX_RING on AF_PACKET "
 				"socket for %s\n", name, pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 
 		rc = setsockopt(qsockfd, SOL_PACKET, PACKET_TX_RING, req, sizeof(*req));
@@ -583,7 +588,7 @@ rte_pmd_init_internals(const char *name,
 			RTE_LOG(ERR, PMD,
 				"%s: could not set PACKET_TX_RING on AF_PACKET "
 				"socket for %s\n", name, pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 
 		rx_queue = &((*internals)->rx_queue[q]);
@@ -596,13 +601,15 @@ rte_pmd_init_internals(const char *name,
 			RTE_LOG(ERR, PMD,
 				"%s: call to mmap failed on AF_PACKET socket for %s\n",
 				name, pair->value);
-			goto error;
+			goto error_free_queues;
 		}
 
 		/* rdsize is same for both Tx and Rx */
 		rdsize = req->tp_frame_nr * sizeof(*(rx_queue->rd));
 
 		rx_queue->rd = rte_zmalloc_socket(name, rdsize, 0, numa_node);
+		if (rx_queue->rd == NULL)
+			goto error_free_queues;
 		for (i = 0; i < req->tp_frame_nr; ++i) {
 			rx_queue->rd[i].iov_base = rx_queue->map + (i * framesize);
 			rx_queue->rd[i].iov_len = req->tp_frame_size;
@@ -615,6 +622,8 @@ rte_pmd_init_internals(const char *name,
 		tx_queue->map = rx_queue->map + req->tp_block_size * req->tp_block_nr;
 
 		tx_queue->rd = rte_zmalloc_socket(name, rdsize, 0, numa_node);
+		if (tx_queue->rd == NULL)
+			goto error_free_queues;
 		for (i = 0; i < req->tp_frame_nr; ++i) {
 			tx_queue->rd[i].iov_base = tx_queue->map + (i * framesize);
 			tx_queue->rd[i].iov_len = req->tp_frame_size;
@@ -671,19 +680,19 @@ rte_pmd_init_internals(const char *name,
 
 	return 0;
 
-error:
-	if (data)
-		rte_free(data);
-	if (pci_dev)
-		rte_free(pci_dev);
+error_free_queues:
+	if (*internals == NULL)
+		return -1;
 	for (q = 0; q < nb_queues; q++) {
-		if ((*internals)->rx_queue[q].rd)
-			rte_free((*internals)->rx_queue[q].rd);
-		if ((*internals)->tx_queue[q].rd)
-			rte_free((*internals)->tx_queue[q].rd);
+		munmap((*internals)->rx_queue[q].map,
+				2 * req->tp_block_size * req->tp_block_nr);
+		rte_free((*internals)->rx_queue[q].rd);
+		rte_free((*internals)->tx_queue[q].rd);
 	}
-	if (*internals)
-		rte_free(*internals);
+error:
+	rte_free(data);
+	rte_free(pci_dev);
+	rte_free(*internals);
 	return -1;
 }
 
-- 
2.1.0

Re: [dpdk-dev] [PATCH] app/test lib/librte_pmd_af_packet: fix check for null & fix possible memory leak

[Thomas Monjalon]

Hi Daniel,

2014-12-17 12:56, Daniel Mrzyglod:
> app/test/test_sched.c:
> -Fix several checking for NULL pointer
> lib/librte_pmd_af_packet/rte_eth_af_packet.c:
> -Not munmaped queue area
> -Fix several checking for NULL pointer

These several fixes should be better explained and referenced
if you could send separate patches.

Thanks
-- 
Thomas