SYS-OSS/FRET-qemu
4
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

- Fix inverted logic in abstract socket QAPI support

Browse Source 
- Only report abstract socket support in QAPI on Linux hosts
  - Expand test coverage
  - Misc other code cleanups
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEE2vOm/bJrYpEtDo4/vobrtBUQT98FAl+hYpUACgkQvobrtBUQ
 T998sw//d3Cd4Ta+zwkMaiziUMmMcO/vhs0X5VLp7HsHZdfAlZKlebI6kmNr8Ucy
 n10bmsIWfrdNMvYpOPyEehbens6BPHELwfw+gkA1rf+7GWS1fafUnz/liRCbITFs
 TglLFtZ9pQfUBOOE42uEkFZrq3Kw4gPn1XYn28PHSLOFQn2Wzy0ww1De/X/EAxUm
 QPcjICB+pPwcavpRC+0Kdz5oxNVXNirc+mPwETZobJ5tGByM5th5O0hT5qmTHcxp
 aWIgllIanGlfJA4DPI5ee92D7voSaen6470kfKY9dcUAopHPZWih0SNv++X9pOFz
 3iSCUOSEkx39ItyXmObeoBUxMXrQxRqyPg3HRGet3TqlpNjsc8t2UiiLWa2OijT5
 wAc2btLoSghtzJvKSYcE9uLJBQq7VbntDvuKO2RZLrraTSUvzlPO57yPIcvrsM3p
 c6Qj5QNwdvsurS+Nqz2a5ATDjLPeZyOcePeVoIYu60wLcL38fEFK8OjHZIqWWlrY
 GEDv+krTa4DhFV5E5OSgQPnZQmWUHSCmCFzF9vyfZpYSUh2OrB05hSi4GhPbAwA1
 n8MZHIG6LHw53Xy9J16l1bInNPfl/9LEiFB4GwjATcZeNEeKp1BKHtu7yFmgwoAh
 DTvKBrVstAGxNMDQd+uVwHiYo2EcWCK4ZHAcofuhhfagpRs7vTc=
 =UBCe
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/berrange-gitlab/tags/sock-next-pull-request' into staging

 - Fix inverted logic in abstract socket QAPI support
 - Only report abstract socket support in QAPI on Linux hosts
 - Expand test coverage
 - Misc other code cleanups

# gpg: Signature made Tue 03 Nov 2020 14:00:53 GMT
# gpg:                using RSA key DAF3A6FDB26B62912D0E8E3FBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>" [full]
# gpg:                 aka "Daniel P. Berrange <berrange@redhat.com>" [full]
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E  8E3F BE86 EBB4 1510 4FDF

* remotes/berrange-gitlab/tags/sock-next-pull-request:
  sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX
  sockets: Bypass "replace empty @path" for abstract unix sockets
  char-socket: Fix qemu_chr_socket_address() for abstract sockets
  sockets: Fix socket_sockaddr_to_address_unix() for abstract sockets
  sockets: Fix default of UnixSocketAddress member @tight
  test-util-sockets: Test the complete abstract socket matrix
  test-util-sockets: Synchronize properly, don't sleep(1)
  test-util-sockets: Factor out test_socket_unix_abstract_one()
  test-util-sockets: Clean up SocketAddress construction
  test-util-sockets: Correct to set has_abstract, has_tight
  test-util-sockets: Plug file descriptor leak

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2020-11-03 14:40:40 +00:00
commit ad26288899
5 changed files with 157 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
chardev/char-socket.c
View File

@ -443,10 +443,24 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix)
s->is_listen ? ",server" : ""); s->is_listen ? ",server" : "");
break; break;
case SOCKET_ADDRESS_TYPE_UNIX: case SOCKET_ADDRESS_TYPE_UNIX:
return g_strdup_printf("%sunix:%s%s", prefix, {
s->addr->u.q_unix.path, const char *tight = "", *abstract = "";
UnixSocketAddress *sa = &s->addr->u.q_unix;
#ifdef CONFIG_LINUX
if (sa->has_abstract && sa->abstract) {
abstract = ",abstract";
if (sa->has_tight && sa->tight) {
tight = ",tight";
}
}
#endif
return g_strdup_printf("%sunix:%s%s%s%s", prefix, sa->path,
abstract, tight,
s->is_listen ? ",server" : ""); s->is_listen ? ",server" : "");
break; break;
}
case SOCKET_ADDRESS_TYPE_FD: case SOCKET_ADDRESS_TYPE_FD:
return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str,
s->is_listen ? ",server" : ""); s->is_listen ? ",server" : "");
@ -1386,8 +1400,10 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
const char *host = qemu_opt_get(opts, "host"); const char *host = qemu_opt_get(opts, "host");
const char *port = qemu_opt_get(opts, "port"); const char *port = qemu_opt_get(opts, "port");
const char *fd = qemu_opt_get(opts, "fd"); const char *fd = qemu_opt_get(opts, "fd");
#ifdef CONFIG_LINUX
bool tight = qemu_opt_get_bool(opts, "tight", true); bool tight = qemu_opt_get_bool(opts, "tight", true);
bool abstract = qemu_opt_get_bool(opts, "abstract", false); bool abstract = qemu_opt_get_bool(opts, "abstract", false);
#endif
SocketAddressLegacy *addr; SocketAddressLegacy *addr;
ChardevSocket *sock; ChardevSocket *sock;
@ -1439,8 +1455,12 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX; addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX;
q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1); q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
q_unix->path = g_strdup(path); q_unix->path = g_strdup(path);
#ifdef CONFIG_LINUX
q_unix->has_tight = true;
q_unix->tight = tight; q_unix->tight = tight;
q_unix->has_abstract = true;
q_unix->abstract = abstract; q_unix->abstract = abstract;
#endif
} else if (host) { } else if (host) {
addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET; addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
addr->u.inet.data = g_new(InetSocketAddress, 1); addr->u.inet.data = g_new(InetSocketAddress, 1);

2
chardev/char.c
View File

@ -928,6 +928,7 @@ QemuOptsList qemu_chardev_opts = {
},{ },{
.name = "logappend", .name = "logappend",
.type = QEMU_OPT_BOOL, .type = QEMU_OPT_BOOL,
#ifdef CONFIG_LINUX
},{ },{
.name = "tight", .name = "tight",
.type = QEMU_OPT_BOOL, .type = QEMU_OPT_BOOL,
@ -935,6 +936,7 @@ QemuOptsList qemu_chardev_opts = {
},{ },{
.name = "abstract", .name = "abstract",
.type = QEMU_OPT_BOOL, .type = QEMU_OPT_BOOL,
#endif
}, },
{ /* end of list */ } { /* end of list */ }
}, },

14
qapi/sockets.json
View File

@ -74,18 +74,20 @@
# Captures a socket address in the local ("Unix socket") namespace. # Captures a socket address in the local ("Unix socket") namespace.
# #
# @path: filesystem path to use # @path: filesystem path to use
# @tight: pass a socket address length confined to the minimum length of the # @abstract: if true, this is a Linux abstract socket address. @path
# abstract string, rather than the full sockaddr_un record length # will be prefixed by a null byte, and optionally padded
# (only matters for abstract sockets, default true). (Since 5.1) # with null bytes. Defaults to false. (Since 5.1)
# @abstract: whether this is an abstract address, default false. (Since 5.1) # @tight: if false, pad an abstract socket address with enough null
# bytes to make it fill struct sockaddr_un member sun_path.
# Defaults to true. (Since 5.1)
# #
# Since: 1.3 # Since: 1.3
## ##
{ 'struct': 'UnixSocketAddress', { 'struct': 'UnixSocketAddress',
'data': { 'data': {
'path': 'str', 'path': 'str',
'*tight': 'bool', '*abstract': { 'type': 'bool', 'if': 'defined(CONFIG_LINUX)' },
'*abstract': 'bool' } } '*tight': { 'type': 'bool', 'if': 'defined(CONFIG_LINUX)' } } }
## ##
# @VsockSocketAddress: # @VsockSocketAddress:

151
tests/test-util-sockets.c
View File

@ -229,94 +229,105 @@ static void test_socket_fd_pass_num_nocli(void)
} }
#endif #endif
#ifdef __linux__ #ifdef CONFIG_LINUX
static gchar *abstract_sock_name;
static gpointer unix_server_thread_func(gpointer user_data) #define ABSTRACT_SOCKET_VARIANTS 3
{
SocketAddress addr;
Error *err = NULL;
int fd = -1;
int connfd = -1;
struct sockaddr_un un;
socklen_t len = sizeof(un);
addr.type = SOCKET_ADDRESS_TYPE_UNIX; typedef struct {
addr.u.q_unix.path = abstract_sock_name; SocketAddress *server, *client[ABSTRACT_SOCKET_VARIANTS];
addr.u.q_unix.tight = user_data != NULL; bool expect_connect[ABSTRACT_SOCKET_VARIANTS];
addr.u.q_unix.abstract = true; } abstract_socket_matrix_row;
fd = socket_listen(&addr, 1, &err);
g_assert_cmpint(fd, >=, 0);
g_assert(fd_is_socket(fd));
connfd = accept(fd, (struct sockaddr *)&un, &len);
g_assert_cmpint(connfd, !=, -1);
close(fd);
return NULL;
}
static gpointer unix_client_thread_func(gpointer user_data) static gpointer unix_client_thread_func(gpointer user_data)
{ {
SocketAddress addr; abstract_socket_matrix_row *row = user_data;
Error *err = NULL; Error *err = NULL;
int fd = -1; int i, fd;
addr.type = SOCKET_ADDRESS_TYPE_UNIX;
addr.u.q_unix.path = abstract_sock_name;
addr.u.q_unix.tight = user_data != NULL;
addr.u.q_unix.abstract = true;
fd = socket_connect(&addr, &err);
for (i = 0; i < ABSTRACT_SOCKET_VARIANTS; i++) {
if (row->expect_connect[i]) {
fd = socket_connect(row->client[i], &error_abort);
g_assert_cmpint(fd, >=, 0); g_assert_cmpint(fd, >=, 0);
} else {
fd = socket_connect(row->client[i], &err);
g_assert_cmpint(fd, ==, -1);
error_free_or_abort(&err);
}
close(fd); close(fd);
}
return NULL; return NULL;
} }
static void test_socket_unix_abstract_good(void) static void test_socket_unix_abstract_row(abstract_socket_matrix_row *test)
{ {
GRand *r = g_rand_new(); int fd, connfd, i;
GThread *cli;
struct sockaddr_un un;
socklen_t len = sizeof(un);
abstract_sock_name = g_strdup_printf("unix-%d-%d", getpid(), /* Last one must connect, or else accept() below hangs */
g_rand_int_range(r, 100, 1000)); assert(test->expect_connect[ABSTRACT_SOCKET_VARIANTS - 1]);
/* non tight socklen serv and cli */ fd = socket_listen(test->server, 1, &error_abort);
GThread *serv = g_thread_new("abstract_unix_server", g_assert_cmpint(fd, >=, 0);
unix_server_thread_func, g_assert(fd_is_socket(fd));
NULL);
sleep(1);
GThread *cli = g_thread_new("abstract_unix_client",
unix_client_thread_func,
NULL);
g_thread_join(cli);
g_thread_join(serv);
/* tight socklen serv and cli */
serv = g_thread_new("abstract_unix_server",
unix_server_thread_func,
(gpointer)1);
sleep(1);
cli = g_thread_new("abstract_unix_client", cli = g_thread_new("abstract_unix_client",
unix_client_thread_func, unix_client_thread_func,
(gpointer)1); test);
for (i = 0; i < ABSTRACT_SOCKET_VARIANTS; i++) {
if (test->expect_connect[i]) {
connfd = accept(fd, (struct sockaddr *)&un, &len);
g_assert_cmpint(connfd, !=, -1);
close(connfd);
}
}
close(fd);
g_thread_join(cli); g_thread_join(cli);
g_thread_join(serv);
g_free(abstract_sock_name);
g_rand_free(r);
} }
#endif
static void test_socket_unix_abstract(void)
{
SocketAddress addr, addr_tight, addr_padded;
abstract_socket_matrix_row matrix[ABSTRACT_SOCKET_VARIANTS] = {
{ &addr,
{ &addr_tight, &addr_padded, &addr },
{ true, false, true } },
{ &addr_tight,
{ &addr_padded, &addr, &addr_tight },
{ false, true, true } },
{ &addr_padded,
{ &addr, &addr_tight, &addr_padded },
{ false, false, true } }
};
int i;
addr.type = SOCKET_ADDRESS_TYPE_UNIX;
addr.u.q_unix.path = g_strdup_printf("unix-%d-%u",
getpid(), g_random_int());
addr.u.q_unix.has_abstract = true;
addr.u.q_unix.abstract = true;
addr.u.q_unix.has_tight = false;
addr.u.q_unix.tight = false;
addr_tight = addr;
addr_tight.u.q_unix.has_tight = true;
addr_tight.u.q_unix.tight = true;
addr_padded = addr;
addr_padded.u.q_unix.has_tight = true;
addr_padded.u.q_unix.tight = false;
for (i = 0; i < ABSTRACT_SOCKET_VARIANTS; i++) {
test_socket_unix_abstract_row(&matrix[i]);
}
g_free(addr.u.q_unix.path);
}
#endif /* CONFIG_LINUX */
int main(int argc, char **argv) int main(int argc, char **argv)
{ {
@ -358,9 +369,9 @@ int main(int argc, char **argv)
#endif #endif
} }
#ifdef __linux__ #ifdef CONFIG_LINUX
g_test_add_func("/util/socket/unix-abstract/good", g_test_add_func("/util/socket/unix-abstract",
test_socket_unix_abstract_good); test_socket_unix_abstract);
#endif #endif
end: end:

54
util/qemu-sockets.c
View File

@ -860,10 +860,29 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
#ifndef _WIN32 #ifndef _WIN32
static bool saddr_is_abstract(UnixSocketAddress *saddr)
{
#ifdef CONFIG_LINUX
return saddr->abstract;
#else
return false;
#endif
}
static bool saddr_is_tight(UnixSocketAddress *saddr)
{
#ifdef CONFIG_LINUX
return !saddr->has_tight || saddr->tight;
#else
return false;
#endif
}
static int unix_listen_saddr(UnixSocketAddress *saddr, static int unix_listen_saddr(UnixSocketAddress *saddr,
int num, int num,
Error **errp) Error **errp)
{ {
bool abstract = saddr_is_abstract(saddr);
struct sockaddr_un un; struct sockaddr_un un;
int sock, fd; int sock, fd;
char *pathbuf = NULL; char *pathbuf = NULL;
@ -877,7 +896,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
return -1; return -1;
} }
if (saddr->path && saddr->path[0]) { if (saddr->path[0] || abstract) {
path = saddr->path; path = saddr->path;
} else { } else {
const char *tmpdir = getenv("TMPDIR"); const char *tmpdir = getenv("TMPDIR");
@ -887,10 +906,10 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
pathlen = strlen(path); pathlen = strlen(path);
if (pathlen > sizeof(un.sun_path) || if (pathlen > sizeof(un.sun_path) ||
(saddr->abstract && pathlen > (sizeof(un.sun_path) - 1))) { (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
error_setg(errp, "UNIX socket path '%s' is too long", path); error_setg(errp, "UNIX socket path '%s' is too long", path);
error_append_hint(errp, "Path must be less than %zu bytes\n", error_append_hint(errp, "Path must be less than %zu bytes\n",
saddr->abstract ? sizeof(un.sun_path) - 1 : abstract ? sizeof(un.sun_path) - 1 :
sizeof(un.sun_path)); sizeof(un.sun_path));
goto err; goto err;
} }
@ -912,7 +931,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
close(fd); close(fd);
} }
if (!saddr->abstract && unlink(path) < 0 && errno != ENOENT) { if (!abstract && unlink(path) < 0 && errno != ENOENT) {
error_setg_errno(errp, errno, error_setg_errno(errp, errno,
"Failed to unlink socket %s", path); "Failed to unlink socket %s", path);
goto err; goto err;
@ -922,10 +941,10 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
un.sun_family = AF_UNIX; un.sun_family = AF_UNIX;
addrlen = sizeof(un); addrlen = sizeof(un);
if (saddr->abstract) { if (abstract) {
un.sun_path[0] = '\0'; un.sun_path[0] = '\0';
memcpy(&un.sun_path[1], path, pathlen); memcpy(&un.sun_path[1], path, pathlen);
if (saddr->tight) { if (saddr_is_tight(saddr)) {
addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen; addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
} }
} else { } else {
@ -952,6 +971,7 @@ err:
static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp) static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
{ {
bool abstract = saddr_is_abstract(saddr);
struct sockaddr_un un; struct sockaddr_un un;
int sock, rc; int sock, rc;
size_t pathlen; size_t pathlen;
@ -970,10 +990,10 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
pathlen = strlen(saddr->path); pathlen = strlen(saddr->path);
if (pathlen > sizeof(un.sun_path) || if (pathlen > sizeof(un.sun_path) ||
(saddr->abstract && pathlen > (sizeof(un.sun_path) - 1))) { (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
error_setg(errp, "UNIX socket path '%s' is too long", saddr->path); error_setg(errp, "UNIX socket path '%s' is too long", saddr->path);
error_append_hint(errp, "Path must be less than %zu bytes\n", error_append_hint(errp, "Path must be less than %zu bytes\n",
saddr->abstract ? sizeof(un.sun_path) - 1 : abstract ? sizeof(un.sun_path) - 1 :
sizeof(un.sun_path)); sizeof(un.sun_path));
goto err; goto err;
} }
@ -982,10 +1002,10 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
un.sun_family = AF_UNIX; un.sun_family = AF_UNIX;
addrlen = sizeof(un); addrlen = sizeof(un);
if (saddr->abstract) { if (abstract) {
un.sun_path[0] = '\0'; un.sun_path[0] = '\0';
memcpy(&un.sun_path[1], saddr->path, pathlen); memcpy(&un.sun_path[1], saddr->path, pathlen);
if (saddr->tight) { if (saddr_is_tight(saddr)) {
addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen; addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
} }
} else { } else {
@ -1270,10 +1290,20 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
addr = g_new0(SocketAddress, 1); addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_TYPE_UNIX; addr->type = SOCKET_ADDRESS_TYPE_UNIX;
if (su->sun_path[0]) { #ifdef CONFIG_LINUX
addr->u.q_unix.path = g_strndup(su->sun_path, sizeof(su->sun_path)); if (!su->sun_path[0]) {
/* Linux abstract socket */
addr->u.q_unix.path = g_strndup(su->sun_path + 1,
sizeof(su->sun_path) - 1);
addr->u.q_unix.has_abstract = true;
addr->u.q_unix.abstract = true;
addr->u.q_unix.has_tight = true;
addr->u.q_unix.tight = salen < sizeof(*su);
return addr;
} }
#endif
addr->u.q_unix.path = g_strndup(su->sun_path, sizeof(su->sun_path));
return addr; return addr;
} }
#endif /* WIN32 */ #endif /* WIN32 */