From: David Goulet <dgoulet@efficios.com>
Date: Tue, 23 Apr 2013 16:34:24 +0000 (-0400)
Subject: Fix: handle invalid ops pointer in relayd_close()
X-Git-Tag: v2.1.2~3
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=6d4c7a40b1050245ed51c198189d47b4b3183c43;p=lttng-tools.git

Fix: handle invalid ops pointer in relayd_close()

Add a lttcomm socket ops pointer check before calling the close
operation of the socket. This can happen if the socket object was
allocated but NOT created.

Furthermore, a fallback is added if no ops pointer is found, the
close(3) system call is used.

This commit also fixes the fact that relayd_close could have been called
on a socket with an invalid operation pointer during the relayd object
creation process in an error path.

Fixes #429

Signed-off-by: David Goulet <dgoulet@efficios.com>
---

diff --git a/src/common/consumer.c b/src/common/consumer.c
index 59b0a1ede..b7e2e9b71 100644
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -2711,7 +2711,7 @@ int consumer_add_relayd_socket(int net_seq_idx, int sock_type,
 		struct pollfd *consumer_sockpoll, struct lttcomm_sock *relayd_sock,
 		unsigned int sessiond_id)
 {
-	int fd = -1, ret = -1, relayd_created = 0;
+	int fd = -1, ret = -1, relayd_created = 0, sock_created = 0;
 	enum lttng_error_code ret_code = LTTNG_OK;
 	struct consumer_relayd_sock_pair *relayd;
 
@@ -2779,6 +2779,8 @@ int consumer_add_relayd_socket(int net_seq_idx, int sock_type,
 
 		/* Assign new file descriptor */
 		relayd->control_sock.fd = fd;
+		/* Flag that we have successfully created a socket with a valid fd. */
+		sock_created = 1;
 
 		/*
 		 * Create a session on the relayd and store the returned id. Lock the
@@ -2814,6 +2816,8 @@ int consumer_add_relayd_socket(int net_seq_idx, int sock_type,
 
 		/* Assign new file descriptor */
 		relayd->data_sock.fd = fd;
+		/* Flag that we have successfully created a socket with a valid fd. */
+		sock_created = 1;
 		break;
 	default:
 		ERR("Unknown relayd socket type (%d)", sock_type);
@@ -2843,9 +2847,11 @@ error:
 	}
 
 	if (relayd_created) {
-		/* We just want to cleanup. Ignore ret value. */
-		(void) relayd_close(&relayd->control_sock);
-		(void) relayd_close(&relayd->data_sock);
+		if (sock_created) {
+			/* We just want to close the fd for cleanup. Ignore ret value. */
+			(void) relayd_close(&relayd->control_sock);
+			(void) relayd_close(&relayd->data_sock);
+		}
 		free(relayd);
 	}
 
diff --git a/src/common/relayd/relayd.c b/src/common/relayd/relayd.c
index 63a55c227..9e10c83e0 100644
--- a/src/common/relayd/relayd.c
+++ b/src/common/relayd/relayd.c
@@ -327,15 +327,44 @@ int relayd_connect(struct lttcomm_sock *sock)
 
 /*
  * Close relayd socket with an allocated lttcomm_sock.
+ *
+ * If no socket operations are found, simply return 0 meaning that everything
+ * is fine. Without operations, the socket can not possibly be opened or used.
+ * This is possible if the socket was allocated but not created. However, the
+ * caller could simply use it to store a valid file descriptor for instance
+ * passed over a Unix socket and call this to cleanup but still without a valid
+ * ops pointer.
+ *
+ * Return the close returned value. On error, a negative value is usually
+ * returned back from close(2).
  */
 int relayd_close(struct lttcomm_sock *sock)
 {
+	int ret;
+
 	/* Code flow error. Safety net. */
 	assert(sock);
 
+	/* An invalid fd is fine, return success. */
+	if (sock->fd < 0) {
+		ret = 0;
+		goto end;
+	}
+
 	DBG3("Relayd closing socket %d", sock->fd);
 
-	return sock->ops->close(sock);
+	if (sock->ops) {
+		ret = sock->ops->close(sock);
+	} else {
+		/* Default call if no specific ops found. */
+		ret = close(sock->fd);
+		if (ret < 0) {
+			PERROR("relayd_close default close");
+		}
+	}
+
+end:
+	return ret;
 }
 
 /*