[PATCH ucspi-vsock 2/7] vsock: check socket family before reading sockaddr

Alyssa Ross
2021-03-19T03:01:58Z

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