pceplib: Fixing coverity messages.

Signed-off-by: Javier Garcia <javier.garcia@voltanet.io>

pathd/path_pcep_cli.c

@@ -1046,7 +1046,7 @@ static int path_pcep_cli_pcc_pcc_peer(struct vty *vty, const char *peer_name,
 		XMALLOC(MTYPE_PCEP, sizeof(struct pcc_opts));
 	memcpy(&pcc_opts_copy->addr,
 	       &pce_opts_cli->pce_opts.config_opts.source_ip,
-	       sizeof(struct pcc_opts));
+	       sizeof(pcc_opts_copy->addr));
 	pcc_opts_copy->msd = pcc_msd_g;
 	pcc_opts_copy->port = pce_opts_cli->pce_opts.config_opts.source_port;
 	if (pcep_ctrl_update_pcc_options(pcep_g->fpt, pcc_opts_copy)) {

pceplib/pcep_msg_messages_encoding.c

@@ -129,7 +129,7 @@ void pcep_encode_message(struct pcep_message *message,
 		message_length +=
 			pcep_encode_object(node->data, versioning,
 					   message_buffer + message_length);
-		if (message_length > PCEP_MESSAGE_LENGTH) {
+		if (message_length >= PCEP_MESSAGE_LENGTH) {
 			message->encoded_message = NULL;
 			message->encoded_message_length = 0;
 			return;
@@ -295,6 +295,14 @@ struct pcep_message *pcep_decode_message(const uint8_t *msg_buf)
 
 	msg_length = pcep_decode_msg_header(msg_buf, &msg_version, &msg_flags,
 					    &msg_type);
+	if (msg_length == 0) {
+		pcep_log(LOG_INFO, "%s: Discarding empty message", __func__);
+		return NULL;
+	}
+	if (msg_length >= PCEP_MESSAGE_LENGTH) {
+		pcep_log(LOG_INFO, "%s: Discarding message too big", __func__);
+		return NULL;
+	}
 
 	struct pcep_message *msg =
 		pceplib_calloc(PCEPLIB_MESSAGES, sizeof(struct pcep_message));

pceplib/pcep_msg_objects_encoding.c

@@ -1668,7 +1668,7 @@ struct pcep_object_header *pcep_decode_obj_ro(struct pcep_object_header *hdr,
 
 				ipv6 = pceplib_malloc(PCEPLIB_MESSAGES,
 						      sizeof(struct in6_addr));
-				decode_ipv6(uint32_ptr + LENGTH_4WORDS, ipv6);
+				decode_ipv6(uint32_ptr + 4, ipv6);
 				dll_append(sr_subobj->nai_list, ipv6);
 
 				read_count += LENGTH_8WORDS;
@@ -1684,17 +1684,17 @@ struct pcep_object_header *pcep_decode_obj_ro(struct pcep_object_header *hdr,
 				struct in_addr *ipv4 =
 					pceplib_malloc(PCEPLIB_MESSAGES,
 						       sizeof(struct in_addr));
-				ipv4->s_addr = uint32_ptr[LENGTH_4WORDS];
+				ipv4->s_addr = uint32_ptr[4];
 				dll_append(sr_subobj->nai_list, ipv4);
 
 				ipv6 = pceplib_malloc(PCEPLIB_MESSAGES,
 						      sizeof(struct in6_addr));
-				decode_ipv6(uint32_ptr + LENGTH_5WORDS, ipv6);
+				decode_ipv6(uint32_ptr + 5, ipv6);
 				dll_append(sr_subobj->nai_list, ipv6);
 
 				ipv4 = pceplib_malloc(PCEPLIB_MESSAGES,
 						      sizeof(struct in_addr));
-				ipv4->s_addr = uint32_ptr[LENGTH_9WORDS];
+				ipv4->s_addr = uint32_ptr[9];
 				dll_append(sr_subobj->nai_list, ipv4);
 
 				read_count += LENGTH_10WORDS;

pceplib/pcep_msg_tlvs.c

@@ -401,7 +401,7 @@ pcep_tlv_create_srpag_pol_name(const char *pol_name, uint16_t pol_name_length)
 		(normalize_pcep_tlv_length(pol_name_length) > MAX_POLICY_NAME)
 			? MAX_POLICY_NAME
 			: pol_name_length;
-	memcpy(tlv->name, pol_name, pol_name_length);
+	memcpy(tlv->name, pol_name, length);
 	tlv->name_length = length;
 
 	return tlv;
@@ -456,7 +456,7 @@ pcep_tlv_create_tlv_arbitrary(const char *data, uint16_t data_length,
 	uint16_t length = (data_length > MAX_ARBITRARY_SIZE)
 				  ? MAX_ARBITRARY_SIZE
 				  : data_length;
-	memcpy(tlv->data, data, data_length);
+	memcpy(tlv->data, data, length);
 	tlv->data_length = length;
 	tlv->arbitraty_type = tlv_id;
 

pceplib/pcep_msg_tlvs_encoding.c

@@ -1135,7 +1135,7 @@ struct pcep_object_tlv_header *pcep_decode_tlv_path_setup_type_capability(
 	uint16_t buf_index = normalize_pcep_tlv_length(
 		TLV_HEADER_LENGTH + LENGTH_1WORD + num_psts);
 	while ((tlv->header.encoded_tlv_length - buf_index) > TLV_HEADER_LENGTH
-	       && num_iterations++ > MAX_ITERATIONS) {
+	       && num_iterations++ < MAX_ITERATIONS) {
 		struct pcep_object_tlv_header *sub_tlv =
 			pcep_decode_tlv(tlv_body_buf + buf_index);
 		if (sub_tlv == NULL) {

pceplib/pcep_msg_tools.c

@@ -93,11 +93,11 @@ static const char *object_class_strs[] = {"NOT_IMPLEMENTED0",
 double_linked_list *pcep_msg_read(int sock_fd)
 {
 	int ret;
-	uint8_t buffer[PCEP_MAX_SIZE] = {0};
+	uint8_t buffer[PCEP_MESSAGE_LENGTH] = {0};
 	uint16_t buffer_read = 0;
 
 
-	ret = read(sock_fd, &buffer, PCEP_MAX_SIZE);
+	ret = read(sock_fd, &buffer, PCEP_MESSAGE_LENGTH);
 
 	if (ret < 0) {
 		pcep_log(
@@ -114,13 +114,13 @@ double_linked_list *pcep_msg_read(int sock_fd)
 	double_linked_list *msg_list = dll_initialize();
 	struct pcep_message *msg = NULL;
 
-	while ((ret - buffer_read) >= MESSAGE_HEADER_LENGTH) {
+	while (((uint16_t)ret - buffer_read) >= MESSAGE_HEADER_LENGTH) {
 
 		/* Get the Message header, validate it, and return the msg
 		 * length */
-		int32_t msg_hdr_length =
+		int32_t msg_length =
 			pcep_decode_validate_msg_header(buffer + buffer_read);
-		if (msg_hdr_length < 0) {
+		if (msg_length < 0 || msg_length > PCEP_MESSAGE_LENGTH) {
 			/* If the message header is invalid, we cant keep
 			 * reading since the length may be invalid */
 			pcep_log(
@@ -130,17 +130,26 @@ double_linked_list *pcep_msg_read(int sock_fd)
 			return msg_list;
 		}
 
-		/* Check if the msg_hdr_length is longer than what was read,
+		/* Check if the msg_length is longer than what was read,
 		 * in which case, we need to read the rest of the message. */
-		if ((ret - buffer_read) < msg_hdr_length) {
-			int read_len = (msg_hdr_length - (ret - buffer_read));
+		if ((ret - buffer_read) < msg_length) {
+			int read_len = (msg_length - (ret - buffer_read));
 			int read_ret = 0;
 			pcep_log(
 				LOG_INFO,
 				"%s: pcep_msg_read: Message not fully read! Trying to read %d bytes more, fd [%d]",
 				__func__, read_len, sock_fd);
 
-			read_ret = read(sock_fd, &buffer[ret], read_len);
+			if (PCEP_MESSAGE_LENGTH - ret - buffer_read >= read_len )
+				read_ret =
+					read(sock_fd, &buffer[ret], read_len);
+			else {
+				pcep_log(
+					LOG_ERR,
+					"%s: Trying to read size (%d) offset (%d) in a buff of size (%d)",
+					__func__, read_len, ret, PCEP_MESSAGE_LENGTH);
+				return msg_list;
+			}
 
 			if (read_ret != read_len) {
 				pcep_log(
@@ -152,7 +161,7 @@ double_linked_list *pcep_msg_read(int sock_fd)
 		}
 
 		msg = pcep_decode_message(buffer + buffer_read);
-		buffer_read += msg_hdr_length;
+		buffer_read += msg_length;
 
 		if (msg == NULL) {
 			return msg_list;
@@ -459,7 +468,12 @@ int pcep_msg_send(int sock_fd, struct pcep_message *msg)
 	if (msg == NULL) {
 		return 0;
 	}
+	int msg_length = ntohs(msg->encoded_message_length);
+	if (msg_length > PCEP_MESSAGE_LENGTH) {
+		pcep_log(LOG_ERR, "%s: Not sended, size(% d) exceed max(% d) ",
+			 __func__, msg_length, PCEP_MESSAGE_LENGTH);
+		return 0;
+	}
 
-	return write(sock_fd, msg->encoded_message,
-		     ntohs(msg->encoded_message_length));
+	return write(sock_fd, msg->encoded_message, msg_length);
 }

pceplib/pcep_pcc.c

@@ -273,6 +273,7 @@ void send_pce_report_message(pcep_session *session)
 		pcep_log(LOG_WARNING,
 			 "%s: send_pce_report_message SRP object was NULL",
 			 __func__);
+			dll_destroy_with_data(report_list);
 		return;
 	}
 	dll_append(report_list, obj);
@@ -313,6 +314,7 @@ void send_pce_report_message(pcep_session *session)
 		pcep_log(LOG_WARNING,
 			 "%s: send_pce_report_message LSP object was NULL",
 			 __func__);
+			dll_destroy_with_data(report_list);
 		return;
 	}
 	dll_append(report_list, obj);
@@ -349,6 +351,7 @@ void send_pce_report_message(pcep_session *session)
 		pcep_log(LOG_WARNING,
 			 "%s: send_pce_report_message ERO object was NULL",
 			 __func__);
+			dll_destroy_with_data(report_list);
 		return;
 	}
 	dll_append(report_list, obj);

pceplib/pcep_session_logic.c

@@ -61,7 +61,6 @@ static bool run_session_logic_common()
 	memset(session_logic_handle_, 0, sizeof(pcep_session_logic_handle));
 
 	session_logic_handle_->active = true;
-	session_logic_handle_->session_logic_condition = false;
 	session_logic_handle_->session_list =
 		ordered_list_initialize(pointer_compare_function);
 	session_logic_handle_->session_event_queue = queue_initialize();
@@ -91,6 +90,11 @@ static bool run_session_logic_common()
 		return false;
 	}
 
+	pthread_mutex_lock(&(session_logic_handle_->session_logic_mutex));
+	session_logic_handle_->session_logic_condition = true;
+	pthread_cond_signal(&(session_logic_handle_->session_logic_cond_var));
+	pthread_mutex_unlock(&(session_logic_handle_->session_logic_mutex));
+
 	if (pthread_mutex_init(&(session_logic_handle_->session_list_mutex),
 			       NULL)
 	    != 0) {
@@ -574,24 +578,20 @@ struct pcep_message *create_pcep_open(pcep_session *session)
 		dll_append(
 			tlv_list,
 			pcep_tlv_create_stateful_pce_capability(
+				/* U flag */
 				session->pcc_config
-					.support_stateful_pce_lsp_update, /* U
-									     flag
-									   */
-				session->pcc_config
-					.support_include_db_version, /* S flag
-								      */
-				session->pcc_config
-					.support_lsp_triggered_resync, /* T flag
-									*/
-				session->pcc_config
-					.support_lsp_delta_sync, /* D flag */
-				session->pcc_config
-					.support_pce_triggered_initial_sync, /* F flag */
+					.support_stateful_pce_lsp_update,
+				/* S flag */
+				session->pcc_config.support_include_db_version,
+				/* I flag */
 				session->pcc_config
-					.support_pce_lsp_instantiation)); /* I
-									     flag
-									   */
+					.support_pce_lsp_instantiation,
+			/* T flag */
+			session->pcc_config.support_lsp_triggered_resync,
+			/* D flag */
+			session->pcc_config.support_lsp_delta_sync,
+			/* F flag */
+			session->pcc_config.support_pce_triggered_initial_sync));
 	}
 
 	if (session->pcc_config.support_include_db_version) {

pceplib/pcep_session_logic_loop.c

@@ -98,6 +98,7 @@ int session_logic_msg_ready_handler(void *data, int socket_fd)
 	} else if (msg_list->num_entries == 0) {
 		/* Invalid message received */
 		increment_unknown_message(session);
+		dll_destroy_with_data(msg_list);
 	} else {
 		/* Just logging the first of potentially several messages
 		 * received */

pceplib/pcep_session_logic_states.c

@@ -24,6 +24,7 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
+#include <assert.h>
 
 #include "pcep_msg_encoding.h"
 #include "pcep_session_logic.h"
@@ -461,6 +462,8 @@ void send_reconciled_pcep_open(pcep_session *session,
 	struct pcep_object_open *open_obj =
 		(struct pcep_object_open *)pcep_obj_get(open_msg->obj_list,
 							PCEP_OBJ_CLASS_OPEN);
+	// open_msg can not have empty obj_list
+	assert(open_obj != NULL);
 
 	if (error_open_obj->open_deadtimer
 	    != session->pce_config.dead_timer_seconds) {

pceplib/pcep_socket_comm_loop.c

@@ -26,6 +26,7 @@
 #include <stddef.h>
 #include <string.h>
 #include <unistd.h>
+#include <assert.h>
 
 #include "pcep_socket_comm_internals.h"
 #include "pcep_socket_comm_loop.h"
@@ -129,6 +130,9 @@ int build_fd_sets(pcep_socket_comm_handle *socket_comm_handle)
 		comm_session = (pcep_socket_comm_session *)node->data;
 		if (comm_session->socket_fd > max_fd) {
 			max_fd = comm_session->socket_fd;
+		} else if (comm_session->socket_fd < 0) {
+			pcep_log(LOG_ERR, "%s: Negative fd", __func__);
+			assert(comm_session->socket_fd > 0);
 		}
 
 		/*pcep_log(LOG_DEBUG, ld] socket_comm::build_fdSets set
@@ -147,6 +151,9 @@ int build_fd_sets(pcep_socket_comm_handle *socket_comm_handle)
 		comm_session = (pcep_socket_comm_session *)node->data;
 		if (comm_session->socket_fd > max_fd) {
 			max_fd = comm_session->socket_fd;
+		} else if (comm_session->socket_fd < 0) {
+			pcep_log(LOG_ERR, "%s: Negative fd", __func__);
+			assert(comm_session->socket_fd > 0);
 		}
 
 		/*pcep_log(LOG_DEBUG, "%s: [%ld] socket_comm::build_fdSets set

pceplib/test/pcep_msg_messages_test.c

@@ -111,7 +111,7 @@ void test_pcep_msg_create_request()
 	/* Test IPv4 */
 	struct pcep_object_rp *rp_obj =
 		pcep_obj_create_rp(0, false, false, false, false, 10, NULL);
-	struct in_addr src_addr, dst_addr;
+	struct in_addr src_addr={}, dst_addr={};
 	struct pcep_object_endpoints_ipv4 *ipv4_obj =
 		pcep_obj_create_endpoint_ipv4(&src_addr, &dst_addr);
 	message = pcep_msg_create_request(rp_obj, ipv4_obj, NULL);
@@ -375,6 +375,10 @@ void test_pcep_msg_create_update()
 	/* Should return NULL if obj_list is empty */
 	message = pcep_msg_create_update(obj_list);
 	CU_ASSERT_PTR_NULL(message);
+	if (message != NULL) {
+		pcep_msg_free_message(message);
+		message = NULL;
+	}
 
 	struct pcep_object_srp *srp = pcep_obj_create_srp(false, 100, NULL);
 	struct pcep_object_lsp *lsp =
@@ -390,6 +394,10 @@ void test_pcep_msg_create_update()
 	CU_ASSERT_PTR_NULL(message);
 
 	dll_append(obj_list, ero);
+	if (message != NULL) {
+		pcep_msg_free_message(message);
+		message = NULL;
+	}
 	message = pcep_msg_create_update(obj_list);
 	CU_ASSERT_PTR_NOT_NULL(message);
 	pcep_encode_message(message, versioning);
@@ -416,6 +424,10 @@ void test_pcep_msg_create_initiate()
 	/* Should return NULL if obj_list is empty */
 	struct pcep_message *message = pcep_msg_create_initiate(NULL);
 	CU_ASSERT_PTR_NULL(message);
+	if (message != NULL) {
+		pcep_msg_free_message(message);
+		message = NULL;
+	}
 
 	struct pcep_object_srp *srp = pcep_obj_create_srp(false, 100, NULL);
 	struct pcep_object_lsp *lsp =
@@ -428,6 +440,10 @@ void test_pcep_msg_create_initiate()
 	dll_append(obj_list, srp);
 	message = pcep_msg_create_initiate(obj_list);
 	CU_ASSERT_PTR_NULL(message);
+	if (message != NULL) {
+		pcep_msg_free_message(message);
+		message = NULL;
+	}
 
 	dll_append(obj_list, lsp);
 	dll_append(obj_list, ero);