patches and low-level development discussion
 help / color / mirror / code / Atom feed
From: Alyssa Ross <hi@alyssa.is>
To: devel@spectrum-os.org
Subject: [PATCH ucspi-vsock 2/7] vsock: check socket family before reading sockaddr
Date: Fri, 19 Mar 2021 02:56:45 +0000	[thread overview]
Message-ID: <20210319025648.17925-2-hi@alyssa.is> (raw)
In-Reply-To: <20210319025349.8839-2-hi@alyssa.is>

Extracting a helper function for this has the nice side effect of
making the `cid' and `port' parameters to vsock_accept nullable, which
is nice for consistency with vsock_get_cid_and_port.
---
This didn't matter so much in the world where the same program was 
responsible for both creating the socket and accepting connections on 
it, but now that we're splitting those up it's important that 
vsockserverd validates its input.

 vsock.c | 31 +++++++++++++++++++++++--------
 vsock.h | 11 ++++++++++-
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/vsock.c b/vsock.c
index 99945c3..6f1f466 100644
--- a/vsock.c
+++ b/vsock.c
@@ -5,6 +5,7 @@
 
 #include "vsock.h"
 
+#include <errno.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 
@@ -17,6 +18,25 @@ static void fill_sockaddr(struct sockaddr_vm *addr, uint32_t cid, uint32_t port)
 	addr->svm_port = port;
 }
 
+static int fill_cid_and_port(const struct sockaddr_vm *addr,
+			     uint32_t *cid, uint32_t *port)
+{
+	// Check that this sockaddr info is actually for the socket
+	// type we think it is, or we could get some very confusing
+	// data out of it.
+	if (addr->svm_family != AF_VSOCK) {
+		errno = EPROTOTYPE;
+		return -1;
+	}
+
+	if (cid)
+		*cid = addr->svm_cid;
+	if (port)
+		*port = addr->svm_port;
+
+	return 0;
+}
+
 int vsock_bind(int fd, uint32_t cid, uint32_t port)
 {
 	struct sockaddr_vm addr = { 0 };
@@ -37,8 +57,8 @@ int vsock_accept(int sockfd, uint32_t *cid, uint32_t *port)
 	if ((fd = accept(sockfd, (struct sockaddr *)&addr, &addr_size)) == -1)
 		return -1;
 
-	*cid = addr.svm_cid;
-	*port = addr.svm_port;
+	if (fill_cid_and_port(&addr, cid, port) == -1)
+		return -1;
 
 	return fd;
 }
@@ -70,10 +90,5 @@ int vsock_get_cid_and_port(int fd, uint32_t *cid, uint32_t *port)
 	if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
 		return -1;
 
-	if (cid)
-		*cid = addr.svm_cid;
-	if (port)
-		*port = addr.svm_port;
-
-	return 0;
+	return fill_cid_and_port(&addr, cid, port);
 }
diff --git a/vsock.h b/vsock.h
index e7ffd62..0b7d157 100644
--- a/vsock.h
+++ b/vsock.h
@@ -3,11 +3,20 @@
 
 #include <stdint.h>
 
+// All functions taking `cid'/`port' output parameters will fail with
+// EPROTOTYPE if the given file descriptor is not an AF_VSOCK socket.
+// The socket is left in an undefined state after this.
+//
+// `cid'/`port' output parameters can be NULL if the information is
+// not required.
+//
+// A return value of -1 indicates failure.  `errno' can be consulted for
+// further information.
+
 int vsock_bind(int fd, uint32_t cid, uint32_t port);
 int vsock_accept(int sockfd, uint32_t *cid, uint32_t *port);
 
 int vsock_connect(int fd, uint32_t cid, uint32_t port);
 int vsock_open(uint32_t cid, uint32_t port);
 
-// `cid' and `port' can be null.
 int vsock_get_cid_and_port(int fd, uint32_t *cid, uint32_t *port);
-- 
2.30.0

  parent reply	other threads:[~2021-03-19  3:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-19  3:23 Broken threading fix Alyssa Ross
2021-03-19  2:56 ` [PATCH ucspi-vsock 1/7] vsock: get cid and port instead of just cid Alyssa Ross
2021-03-19  2:53   ` [PATCH ucspi-vsock 0/7] Extract vsockserver-socketbinder and vsockserverd Alyssa Ross
2021-03-19  3:19     ` Alyssa Ross
2021-03-19  2:56 ` Alyssa Ross [this message]
2021-03-19  2:56 ` [PATCH ucspi-vsock 3/7] vsockserver: switch to a non-blocking socket Alyssa Ross
2021-03-19  2:56 ` [PATCH ucspi-vsock 4/7] configure: create, to generate config.h Alyssa Ross
2021-03-19  2:56 ` [PATCH ucspi-vsock 5/7] log: add die function Alyssa Ross
2021-03-19  2:56 ` [PATCH ucspi-vsock 6/7] exec: add execzp() Alyssa Ross
2021-03-19  3:17 ` [PATCH ucspi-vsock 7/7] Extract vsockserver-socketbinder and vsockserverd Alyssa Ross
2021-03-19  3:39   ` Cole Helbling
2021-03-20 20:24     ` Alyssa Ross
2021-03-21  2:52       ` Cole Helbling
2021-03-21 14:33         ` Alyssa Ross
2021-03-21 14:38           ` Alyssa Ross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210319025648.17925-2-hi@alyssa.is \
    --to=hi@alyssa.is \
    --cc=devel@spectrum-os.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://spectrum-os.org/git/crosvm
	https://spectrum-os.org/git/doc
	https://spectrum-os.org/git/mktuntap
	https://spectrum-os.org/git/nixpkgs
	https://spectrum-os.org/git/spectrum
	https://spectrum-os.org/git/ucspi-vsock
	https://spectrum-os.org/git/www

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).