Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
[LibOS] Relax check on UNIX domain socket's addrlen in bind and `…
Browse files Browse the repository at this point in the history 
…connect`

Previously, LibOS expected `addrlen` for UNIX domain sockets to be
exactly the same as the size of `sockaddr_un` struct. This is not true
in the common case: `addrlen` may be less, see `man unix`.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
  • Loading branch information
@mkow
Dmitrii Kuvaiskii authored and mkow committed Aug 11, 2021
1 parent fbc8e45 commit 1a2275e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 11 deletions.
50 changes: 40 additions & 10 deletions LibOS/shim/src/sys/shim_socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,17 +465,32 @@ long shim_do_bind(int sockfd, struct sockaddr* addr, int _addrlen) {
}

if (sock->domain == AF_UNIX) {
if (addrlen != sizeof(struct sockaddr_un))
if (addrlen < offsetof(struct sockaddr_un, sun_path) + 1 ||
addrlen > sizeof(struct sockaddr_un)) {
/* address length of the UNIX domain socket is typically calculated as
* `offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1`, see `man unix` */
goto out;
}

struct sockaddr_un* saddr = (struct sockaddr_un*)addr;
char* spath = saddr->sun_path;
struct shim_dentry* dent = NULL;
if (saddr->sun_path[0] == '\0') {
/* currently abstract UNIX domain sockets are not supported */
ret = -EOPNOTSUPP;
goto out;
}

if ((ret = path_lookupat(/*start=*/NULL, spath, LOOKUP_NO_FOLLOW | LOOKUP_CREATE, &dent)) < 0) {
size_t sun_path_size = addrlen - offsetof(struct sockaddr_un, sun_path);
if (strnlen(saddr->sun_path, sun_path_size) == sun_path_size) {
/* user-supplied UNIX domain name is not a NULL-terminating string */
goto out;
}

struct shim_dentry* dent = NULL;
ret = path_lookupat(/*start=*/NULL, saddr->sun_path, LOOKUP_NO_FOLLOW | LOOKUP_CREATE,
&dent);
if (ret < 0)
goto out;

if (!(dent->state & DENTRY_NEGATIVE)) {
ret = -EADDRINUSE;
goto out;
Expand Down Expand Up @@ -517,7 +532,7 @@ long shim_do_bind(int sockfd, struct sockaddr* addr, int _addrlen) {
if (sock->domain == AF_UNIX) {
struct shim_dentry* dent = sock->addr.un.dentry;

dent->state ^= DENTRY_NEGATIVE;
dent->state &= ~DENTRY_NEGATIVE;
dent->state |= DENTRY_VALID;
dent->fs = &socket_builtin_fs;
dent->type = S_IFSOCK;
Expand Down Expand Up @@ -733,17 +748,32 @@ long shim_do_connect(int sockfd, struct sockaddr* addr, int _addrlen) {
}

if (sock->domain == AF_UNIX) {
if (addrlen != sizeof(struct sockaddr_un))
if (addrlen < offsetof(struct sockaddr_un, sun_path) + 1 ||
addrlen > sizeof(struct sockaddr_un)) {
/* address length of the UNIX domain socket is typically calculated as
* `offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1`, see `man unix` */
goto out;
}

struct sockaddr_un* saddr = (struct sockaddr_un*)addr;
char* spath = saddr->sun_path;
struct shim_dentry* dent = NULL;
if (saddr->sun_path[0] == '\0') {
/* currently abstract UNIX domain sockets are not supported */
ret = -EOPNOTSUPP;
goto out;
}

if ((ret = path_lookupat(/*start=*/NULL, spath, LOOKUP_CREATE | LOOKUP_FOLLOW, &dent)) < 0) {
size_t sun_path_size = addrlen - offsetof(struct sockaddr_un, sun_path);
if (strnlen(saddr->sun_path, sun_path_size) == sun_path_size) {
/* user-supplied UNIX domain name is not a NULL-terminating string */
goto out;
}

struct shim_dentry* dent = NULL;
ret = path_lookupat(/*start=*/NULL, saddr->sun_path, LOOKUP_FOLLOW | LOOKUP_CREATE,
&dent);
if (ret < 0)
goto out;

if (!(dent->state & DENTRY_NEGATIVE) && dent->type != S_IFSOCK) {
ret = -ECONNREFUSED;
put_dentry(dent);
Expand Down Expand Up @@ -793,7 +823,7 @@ long shim_do_connect(int sockfd, struct sockaddr* addr, int _addrlen) {
if (sock->domain == AF_UNIX) {
struct shim_dentry* dent = sock->addr.un.dentry;
lock(&dent->lock);
dent->state ^= DENTRY_NEGATIVE;
dent->state &= ~DENTRY_NEGATIVE;
dent->state |= DENTRY_VALID;
dent->fs = &socket_builtin_fs;
dent->type = S_IFSOCK;
Expand Down
5 changes: 5 additions & 0 deletions LibOS/shim/test/regression/test_libos.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,11 @@ def test_095_mkfifo(self):
self.assertIn('[parent] TEST OK', stdout)

def test_100_socket_unix(self):
if os.path.exists("dummy"):
os.remove("dummy")
if os.path.exists("u"):
os.remove("u")

stdout, _ = self.run_binary(['unix'])
self.assertIn('Data: This is packet 0', stdout)
self.assertIn('Data: This is packet 1', stdout)
Expand Down
5 changes: 4 additions & 1 deletion LibOS/shim/test/regression/unix.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -47,8 +48,10 @@ static int server_dummy_socket(void) {

address.sun_family = AF_UNIX;
strncpy(address.sun_path, "dummy", sizeof(address.sun_path));
socklen_t minimal_address_size = offsetof(struct sockaddr_un, sun_path) +
strlen(address.sun_path) + 1;

if (bind(create_socket, (struct sockaddr*)&address, sizeof(address)) < 0) {
if (bind(create_socket, (struct sockaddr*)&address, minimal_address_size) < 0) {
perror("bind");
close(create_socket);
exit(1);
Expand Down

0 comments on commit 1a2275e

Please sign in to comment.