OpenE2K
/
qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

sockets: Fix default of UnixSocketAddress member @tight 

An optional bool member of a QAPI struct can be false, true, or absent.
The previous commit demonstrated that socket_listen() and
socket_connect() are broken for absent @tight, and indeed QMP chardev-
add also defaults absent member @tight to false instead of true.

In C, QAPI members are represented by two fields, has_MEMBER and MEMBER.
We have:

            has_MEMBER    MEMBER
    false         true     false
    true          true      true
    absent       false  false/ignore

When has_MEMBER is false, MEMBER should be set to false on write, and
ignored on read.

For QMP, the QAPI visitors handle absent @tight by setting both
@has_tight and @tight to false.  unix_listen_saddr() and
unix_connect_saddr() however use @tight only, disregarding @has_tight.
This is wrong and means that absent @tight defaults to false whereas it
should default to true.

The same is true for @has_abstract, though @abstract defaults to
false and therefore has the same behavior for all of QMP, HMP and CLI.
Fix unix_listen_saddr() and unix_connect_saddr() to check
@has_abstract/@has_tight, and to default absent @tight to true.

However, this is only half of the story.  HMP chardev-add and CLI
-chardev so far correctly defaulted @tight to true, but defaults to
false again with the above fix for HMP and CLI.  In fact, the "tight"
and "abstract" options now break completely.

Digging deeper, we find that qemu_chr_parse_socket() also ignores
@has_tight, leaving it false when it sets @tight.  That is also wrong,
but the two wrongs cancelled out.  Fix qemu_chr_parse_socket() to set
@has_tight and @has_abstract; writing testcases for HMP and CLI is left
for another day.

Fixes: 776b97d360
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Markus Armbruster 2020-11-02 10:44:18 +01:00 committed by Daniel P. Berrangé
parent a72f6754a1
commit b08cc97d6b
3 changed files with 7 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
chardev/char-socket.c
View File

@ -1439,7 +1439,9 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX;
q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
q_unix->path = g_strdup(path);
q_unix->has_tight = true;
q_unix->tight = tight;
q_unix->has_abstract = true;
q_unix->abstract = abstract;
} else if (host) {
addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;

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

@ -294,13 +294,13 @@ static void test_socket_unix_abstract(void)
abstract_socket_matrix_row matrix[ABSTRACT_SOCKET_VARIANTS] = {
{ &addr,
{ &addr_tight, &addr_padded, &addr },
{ false /* BUG */, true /* BUG */, true } },
{ true, false, true } },
{ &addr_tight,
{ &addr_padded, &addr, &addr_tight },
{ false, false /* BUG */, true } },
{ false, true, true } },
{ &addr_padded,
{ &addr, &addr_tight, &addr_padded },
{ true /* BUG */, false, true } }
{ false, false, true } }
};
int i;

4
util/qemu-sockets.c
View File

@ -925,7 +925,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
if (saddr->abstract) {
un.sun_path[0] = '\0';
memcpy(&un.sun_path[1], path, pathlen);
if (saddr->tight) {
if (!saddr->has_tight || saddr->tight) {
addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
}
} else {
@ -985,7 +985,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
if (saddr->abstract) {
un.sun_path[0] = '\0';
memcpy(&un.sun_path[1], saddr->path, pathlen);
if (saddr->tight) {
if (!saddr->has_tight || saddr->tight) {
addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
}
} else {