[PATCH] ipcc: Check that ipc socket is correctly connected

[Jan Friesse <jfriesse@xxxxxxxxxx>]

If client application closes fd returned by corosync ipc, various
strange things can happen. Easiest way how to prevent such situation is
to properly check if fd is valid.

Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
---
 lib/coroipcc.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/lib/coroipcc.c b/lib/coroipcc.c
index 3ad4921..718a2aa 100644
--- a/lib/coroipcc.c
+++ b/lib/coroipcc.c
@@ -59,6 +59,7 @@
 #include <sys/mman.h>
 
 #include <corosync/corotypes.h>
+#include <corosync/corodefs.h>
 #include <corosync/coroipc_types.h>
 #include <corosync/coroipc_ipc.h>
 #include <corosync/coroipcc.h>
@@ -96,6 +97,7 @@ struct ipc_path_data {
 };
 
 void ipc_hdb_destructor (void *context);
+static int is_valid_ipc_fd(int fd);
 
 DECLARE_HDB_DATABASE(ipc_hdb,ipc_hdb_destructor);
 
@@ -268,6 +270,11 @@ priv_change_send (struct ipc_instance *ipc_instance)
 	if (ipc_instance->euid == req_priv_change.euid) {
 		return (0);
 	}
+
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 	req_priv_change.egid = getegid();
 
 	buf_req = MESSAGE_REQ_CHANGE_EUID;
@@ -526,6 +533,10 @@ reply_receive (
 	coroipc_response_header_t *response_header;
 	cs_error_t res;
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 retry_ipc_sem_wait:
 	res = ipc_sem_wait (ipc_instance->control_buffer, SEMAPHORE_RESPONSE, ipc_instance->fd);
 	if (res != CS_OK) {
@@ -553,6 +564,10 @@ reply_receive_in_buf (
 {
 	cs_error_t res;
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 retry_ipc_sem_wait:
 	res = ipc_sem_wait (ipc_instance->control_buffer, SEMAPHORE_RESPONSE, ipc_instance->fd);
 	if (res != CS_OK) {
@@ -569,6 +584,54 @@ retry_ipc_sem_wait:
 }
 
 /*
+ * Check that passed fd is valid socket and it's peer is really corosync socket
+ */
+static int
+is_valid_ipc_fd(int fd)
+{
+	struct sockaddr_un peer_addr;
+	socklen_t peer_len;
+	int res;
+	struct sockaddr_un expected_addr;
+	size_t cmp_len;
+
+	memset(&peer_addr, 0, sizeof(peer_addr));
+	peer_len = sizeof(peer_addr);
+	res = getpeername(fd, &peer_addr, &peer_len);
+	if (res != 0) {
+		/*
+		 * It's not valid socket
+		 */
+		return (0);
+	}
+	if (peer_addr.sun_family != AF_UNIX) {
+		/*
+		 * It's socket but not unix one
+		 */
+		return (0);
+	}
+
+	/*
+	 * Fill expected_addr
+	 */
+	memset(&expected_addr, 0, sizeof(expected_addr));
+#if defined(COROSYNC_LINUX)
+	cmp_len = sprintf(expected_addr.sun_path + 1, "%s", COROSYNC_SOCKET_NAME);
+	cmp_len++;
+#else
+	cmp_len = sprintf(expected_addr.sun_path, "%s/%s", SOCKETDIR, COROSYNC_SOCKET_NAME);
+#endif
+	if (memcmp(peer_addr.sun_path, expected_addr.sun_path, cmp_len) != 0) {
+		/*
+		 * Path is different then expected
+		 */
+		return (0);
+	}
+
+	return (1);
+}
+
+/*
  * External API
  */
 cs_error_t
@@ -641,6 +704,7 @@ coroipcc_service_connect (
 		goto error_connect;
 	}
 
+
 	sys_res = memory_map (
 		path_data->control_map_path,
 		"control_buffer-XXXXXX",
@@ -808,6 +872,10 @@ coroipcc_service_disconnect (
 		return (res);
 	}
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 	shutdown (ipc_instance->fd, SHUT_RDWR);
 	close (ipc_instance->fd);
 	hdb_handle_destroy (&ipc_hdb, handle);
@@ -847,6 +915,10 @@ coroipcc_fd_get (
 		return (res);
 	}
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 	*fd = ipc_instance->fd;
 
 	hdb_handle_put (&ipc_hdb, handle);
@@ -885,6 +957,10 @@ coroipcc_dispatch_get (
 
 	*data = NULL;
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 	ufds.fd = ipc_instance->fd;
 	ufds.events = POLLIN;
 	ufds.revents = 0;
@@ -907,6 +983,10 @@ coroipcc_dispatch_get (
 		goto error_put;
 	}
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 	error = socket_recv (ipc_instance->fd, &buf, 1);
 	if (error != CS_OK) {
 		goto error_put;
@@ -952,6 +1032,10 @@ coroipcc_dispatch_put (hdb_handle_t handle)
 		return (res);
 	}
 
+	if (!is_valid_ipc_fd(ipc_instance->fd)) {
+		return (CS_ERR_BAD_HANDLE);
+	}
+
 retry_ipc_sem_wait:
 	res = ipc_sem_wait (ipc_instance->control_buffer, SEMAPHORE_DISPATCH, ipc_instance->fd);
 	if (res != CS_OK) {
-- 
1.7.1

_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss