From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, berrange@redhat.com, zxq_yx_007@163.com,
kraxel@redhat.com, pbonzini@redhat.com,
marcandre.lureau@redhat.com
Subject: [PATCH v2 07/11] sockets: Fix default of UnixSocketAddress member @tight
Date: Mon, 2 Nov 2020 10:44:18 +0100 [thread overview]
Message-ID: <20201102094422.173975-8-armbru@redhat.com> (raw)
In-Reply-To: <20201102094422.173975-1-armbru@redhat.com>
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: 776b97d3605ed0fc94443048fdf988c7725e38a9
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>
---
chardev/char-socket.c | 2 ++
tests/test-util-sockets.c | 6 +++---
util/qemu-sockets.c | 4 ++--
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 95e45812d5..1ee5a8c295 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -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;
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index f8b6586e70..7ecf95579b 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -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;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 38f82179b0..3ceaa81226 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -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 {
--
2.26.2
next prev parent reply other threads:[~2020-11-02 9:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-02 9:44 [PATCH v2 00/11] sockets: Attempt to drain the abstract socket swamp Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 01/11] test-util-sockets: Plug file descriptor leak Markus Armbruster
2020-11-02 14:10 ` Philippe Mathieu-Daudé
2020-11-02 9:44 ` [PATCH v2 02/11] test-util-sockets: Correct to set has_abstract, has_tight Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 03/11] test-util-sockets: Clean up SocketAddress construction Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 04/11] test-util-sockets: Factor out test_socket_unix_abstract_one() Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 05/11] test-util-sockets: Synchronize properly, don't sleep(1) Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 06/11] test-util-sockets: Test the complete abstract socket matrix Markus Armbruster
2020-11-02 9:44 ` Markus Armbruster [this message]
2020-11-02 9:44 ` [PATCH v2 08/11] sockets: Fix socket_sockaddr_to_address_unix() for abstract sockets Markus Armbruster
2020-11-02 14:04 ` Eric Blake
2020-11-02 9:44 ` [PATCH v2 09/11] char-socket: Fix qemu_chr_socket_address() " Markus Armbruster
2020-11-02 14:08 ` Eric Blake
2020-11-03 6:28 ` Markus Armbruster
2020-11-03 13:17 ` Daniel P. Berrangé
2020-11-03 15:21 ` Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 10/11] sockets: Bypass "replace empty @path" for abstract unix sockets Markus Armbruster
2020-11-02 9:44 ` [PATCH v2 11/11] sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX Markus Armbruster
2020-11-02 14:12 ` Eric Blake
2020-11-03 6:35 ` Markus Armbruster
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=20201102094422.173975-8-armbru@redhat.com \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=kraxel@redhat.com \
--cc=kwolf@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=zxq_yx_007@163.com \
/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.
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).