swc

commit a98426ae69b43396cb22bb62316e270fb1f9d8f1
parent 63d8812d7789bd89d426c57ffb62a2ba95a0abaf
Author: Michael Forney <mforney@mforney.org>
Date:   Wed, 10 Jul 2019 22:51:55 -0700

launch: Use iovec to avoid undefined behavior and VLAs with open device request

Diffstat:
M
launch/launch.c
 | 
40
++++++++++++++++++++++++----------------
M
launch/protocol.c
 | 
38
+++++++++++++-------------------------
M
launch/protocol.h
 | 
7
++++---
M
libswc/launch.c
 | 
32
++++++++++++++++++++------------

4 files changed, 61 insertions(+), 56 deletions(-)
diff --git a/launch/launch.c b/launch/launch.c
@@ -29,6 +29,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <poll.h>
 #include <spawn.h>
 #include <stdbool.h>
@@ -172,30 +173,37 @@ handle_signal(int sig)
 static void
 handle_socket_data(int socket)
 {
-	char buffer[BUFSIZ];
-	struct swc_launch_request *request = (void *)&buffer;
+	struct swc_launch_request request;
 	struct swc_launch_event response;
+	char path[PATH_MAX];
+	struct iovec request_iov[2] = {
+		{.iov_base = &request, .iov_len = sizeof(request)},
+		{.iov_base = path, .iov_len = sizeof(path)},
+	};
+	struct iovec response_iov[1] = {
+		{.iov_base = &response, .iov_len = sizeof(response)},
+	};
 	int fd = -1;
 	struct stat st;
 	ssize_t size;
 
-	size = receive_fd(socket, &fd, buffer, sizeof(buffer));
-
-	if (size == -1 || size == 0)
+	size = receive_fd(socket, &fd, request_iov, 2);
+	if (size == -1 || size == 0 || size < sizeof(request))
 		return;
+	size -= sizeof(request);
 
 	response.type = SWC_LAUNCH_EVENT_RESPONSE;
-	response.serial = request->serial;
+	response.serial = request.serial;
 
-	switch (request->type) {
+	switch (request.type) {
 	case SWC_LAUNCH_REQUEST_OPEN_DEVICE:
-		if (buffer[size - 1] != '\0') {
+		if (size == 0 || path[size - 1] != '\0') {
 			fprintf(stderr, "path is not NULL terminated\n");
 			goto fail;
 		}
 
-		if (stat(request->path, &st) == -1) {
-			fprintf(stderr, "stat %s: %s\n", request->path, strerror(errno));
+		if (stat(path, &st) == -1) {
+			fprintf(stderr, "stat %s: %s\n", path, strerror(errno));
 			goto fail;
 		}
 
@@ -219,10 +227,10 @@ handle_socket_data(int socket)
 			goto fail;
 		}
 
-		fd = open(request->path, request->flags);
+		fd = open(path, request.flags);
 
 		if (fd == -1) {
-			fprintf(stderr, "open %s: %s\n", request->path, strerror(errno));
+			fprintf(stderr, "open %s: %s\n", path, strerror(errno));
 			goto fail;
 		}
 
@@ -240,11 +248,11 @@ handle_socket_data(int socket)
 		if (!active)
 			goto fail;
 
-		if (ioctl(tty_fd, VT_ACTIVATE, request->vt) == -1)
-			fprintf(stderr, "failed to activate VT %d: %s\n", request->vt, strerror(errno));
+		if (ioctl(tty_fd, VT_ACTIVATE, request.vt) == -1)
+			fprintf(stderr, "failed to activate VT %d: %s\n", request.vt, strerror(errno));
 		break;
 	default:
-		fprintf(stderr, "unknown request %u\n", request->type);
+		fprintf(stderr, "unknown request %u\n", request.type);
 		goto fail;
 	}
 
@@ -255,7 +263,7 @@ fail:
 	response.success = false;
 	fd = -1;
 done:
-	send_fd(socket, fd, &response, sizeof(response));
+	send_fd(socket, fd, response_iov, 1);
 }
 
 static void
diff --git a/launch/protocol.c b/launch/protocol.c
@@ -5,18 +5,14 @@
 #include <string.h>
 
 ssize_t
-send_fd(int socket, int fd, const void *buffer, size_t buffer_size)
+send_fd(int socket, int fd, struct iovec *iov, int iovlen)
 {
 	char control[CMSG_SPACE(sizeof(fd))];
-	struct iovec iov = {
-		.iov_base = (void *)buffer,
-		.iov_len = buffer_size,
-	};
 	struct msghdr message = {
 		.msg_name = NULL,
 		.msg_namelen = 0,
-		.msg_iov = &iov,
-		.msg_iovlen = 1,
+		.msg_iov = iov,
+		.msg_iovlen = iovlen,
 	};
 	struct cmsghdr *cmsg;
 
@@ -39,39 +35,31 @@ send_fd(int socket, int fd, const void *buffer, size_t buffer_size)
 }
 
 ssize_t
-receive_fd(int socket, int *fd, void *buffer, size_t buffer_size)
+receive_fd(int socket, int *fd, struct iovec *iov, int iovlen)
 {
-	if (!fd)
-		return recv(socket, buffer, buffer_size, 0);
-
 	ssize_t size;
 	char control[CMSG_SPACE(sizeof(*fd))];
-	struct iovec iov = {
-		.iov_base = buffer,
-		.iov_len = buffer_size,
-	};
 	struct msghdr message = {
 		.msg_name = NULL,
 		.msg_namelen = 0,
-		.msg_iov = &iov,
-		.msg_iovlen = 1,
-		.msg_control = &control,
-		.msg_controllen = sizeof(control),
+		.msg_iov = iov,
+		.msg_iovlen = iovlen,
 	};
 	struct cmsghdr *cmsg;
 
-	*fd = -1;
+	if (fd) {
+		*fd = -1;
+		message.msg_control = &control;
+		message.msg_controllen = sizeof(control);
+	}
+
 	size = recvmsg(socket, &message, 0);
 	if (size < 0)
 		return -1;
 
 	cmsg = CMSG_FIRSTHDR(&message);
-
-	if (cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(*fd)) &&
-	    cmsg->cmsg_level == SOL_SOCKET &&
-	    cmsg->cmsg_type == SCM_RIGHTS) {
+	if (fd && cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(*fd)) && cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS)
 		memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
-	}
 
 	return size;
 }
diff --git a/launch/protocol.h b/launch/protocol.h
@@ -30,6 +30,8 @@
 
 #define SWC_LAUNCH_SOCKET_ENV "SWC_LAUNCH_SOCKET"
 
+struct iovec;
+
 struct swc_launch_request {
 	enum {
 		SWC_LAUNCH_REQUEST_OPEN_DEVICE,
@@ -41,7 +43,6 @@ struct swc_launch_request {
 	union {
 		struct /* OPEN_DEVICE */ {
 			int flags;
-			char path[];
 		};
 		struct /* ACTIVATE_VT */ {
 			unsigned vt;
@@ -64,7 +65,7 @@ struct swc_launch_event {
 	};
 };
 
-ssize_t send_fd(int socket, int fd, const void *buffer, size_t buffer_size);
-ssize_t receive_fd(int socket, int *fd, void *buffer, size_t buffer_size);
+ssize_t send_fd(int socket, int fd, struct iovec *iov, int iovlen);
+ssize_t receive_fd(int socket, int *fd, struct iovec *iov, int iovlen);
 
 #endif
diff --git a/libswc/launch.c b/libswc/launch.c
@@ -58,8 +58,11 @@ static int
 handle_data(int fd, uint32_t mask, void *data)
 {
 	struct swc_launch_event event;
+	struct iovec iov[1] = {
+		{.iov_base = &event, .iov_len = sizeof(event)},
+	};
 
-	if (receive_fd(fd, NULL, &event, sizeof(event)) != -1)
+	if (receive_fd(fd, NULL, iov, 1) != -1)
 		handle_event(&event);
 	return 1;
 }
@@ -95,14 +98,22 @@ launch_finalize(void)
 }
 
 static bool
-send_request(struct swc_launch_request *request, size_t size, struct swc_launch_event *event, int out_fd, int *in_fd)
+send_request(struct swc_launch_request *request, const void *data, size_t size, struct swc_launch_event *event, int out_fd, int *in_fd)
 {
+	struct iovec request_iov[2] = {
+		{.iov_base = request, .iov_len = sizeof(*request)},
+		{.iov_base = (void *)data, .iov_len = size},
+	};
+	struct iovec response_iov[1] = {
+		{.iov_base = event, .iov_len = sizeof(*event)},
+	};
+
 	request->serial = ++launch.next_serial;
 
-	if (send_fd(launch.socket, out_fd, request, size) == -1)
+	if (send_fd(launch.socket, out_fd, request_iov, 1 + (size > 0)) == -1)
 		return false;
 
-	while (receive_fd(launch.socket, in_fd, event, sizeof(*event)) != -1) {
+	while (receive_fd(launch.socket, in_fd, response_iov, 1) != -1) {
 		if (event->type == SWC_LAUNCH_EVENT_RESPONSE && event->serial == request->serial)
 			return true;
 		handle_event(event);
@@ -114,17 +125,14 @@ send_request(struct swc_launch_request *request, size_t size, struct swc_launch_
 int
 launch_open_device(const char *path, int flags)
 {
-	size_t path_size = strlen(path);
-	char buffer[sizeof(struct swc_launch_request) + path_size + 1];
-	struct swc_launch_request *request = (void *)buffer;
+	struct swc_launch_request request;
 	struct swc_launch_event response;
 	int fd;
 
-	request->type = SWC_LAUNCH_REQUEST_OPEN_DEVICE;
-	request->flags = flags;
-	strcpy(request->path, path);
+	request.type = SWC_LAUNCH_REQUEST_OPEN_DEVICE;
+	request.flags = flags;
 
-	if (!send_request(request, sizeof(buffer), &response, -1, &fd))
+	if (!send_request(&request, path, strlen(path) + 1, &response, -1, &fd))
 		return -1;
 
 	return fd;
@@ -139,7 +147,7 @@ launch_activate_vt(unsigned vt)
 	request.type = SWC_LAUNCH_REQUEST_ACTIVATE_VT;
 	request.vt = vt;
 
-	if (!send_request(&request, sizeof(request), &response, -1, NULL))
+	if (!send_request(&request, NULL, 0, &response, -1, NULL))
 		return false;
 
 	return response.success;