From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
The QMP query-vnc interfaces have gained a lot more information that
the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse
connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)
x509_dname: none
sasl_username: none
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)
x509_dname: none
sasl_username: none
Auth: none (Sub: none)
-vnc :1,password,id=pass -vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)
x509_dname: none
sasl_username: none
Auth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)
x509_dname: none
sasl_username: none
This was originally RH bz 1461682
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20170711154414.21111-1-dgilbert@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hmp.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 73 insertions(+), 31 deletions(-)
diff --git a/hmp.c b/hmp.c
index dee40284c1..3e1e766593 100644
--- a/hmp.c
+++ b/hmp.c
@@ -600,50 +600,92 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
qapi_free_BlockStatsList(stats_list);
}
+/* Helper for hmp_info_vnc_clients, _servers */
+static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
+ const char *name)
+{
+ monitor_printf(mon, " %s: %s:%s (%s%s)\n",
+ name,
+ info->host,
+ info->service,
+ NetworkAddressFamily_lookup[info->family],
+ info->websocket ? " (Websocket)" : "");
+}
+
+/* Helper displaying and auth and crypt info */
+static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
+ VncPrimaryAuth auth,
+ VncVencryptSubAuth *vencrypt)
+{
+ monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
+ VncPrimaryAuth_lookup[auth],
+ vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] : "none");
+}
+
+static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
+{
+ while (client) {
+ VncClientInfo *cinfo = client->value;
+
+ hmp_info_VncBasicInfo(mon, qapi_VncClientInfo_base(cinfo), "Client");
+ monitor_printf(mon, " x509_dname: %s\n",
+ cinfo->has_x509_dname ?
+ cinfo->x509_dname : "none");
+ monitor_printf(mon, " sasl_username: %s\n",
+ cinfo->has_sasl_username ?
+ cinfo->sasl_username : "none");
+
+ client = client->next;
+ }
+}
+
+static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
+{
+ while (server) {
+ VncServerInfo2 *sinfo = server->value;
+ hmp_info_VncBasicInfo(mon, qapi_VncServerInfo2_base(sinfo), "Server");
+ hmp_info_vnc_authcrypt(mon, " ", sinfo->auth,
+ sinfo->has_vencrypt ? &sinfo->vencrypt : NULL);
+ server = server->next;
+ }
+}
+
void hmp_info_vnc(Monitor *mon, const QDict *qdict)
{
- VncInfo *info;
+ VncInfo2List *info2l;
Error *err = NULL;
- VncClientInfoList *client;
- info = qmp_query_vnc(&err);
+ info2l = qmp_query_vnc_servers(&err);
if (err) {
error_report_err(err);
return;
}
-
- if (!info->enabled) {
- monitor_printf(mon, "Server: disabled\n");
- goto out;
- }
-
- monitor_printf(mon, "Server:\n");
- if (info->has_host && info->has_service) {
- monitor_printf(mon, " address: %s:%s\n", info->host, info->service);
- }
- if (info->has_auth) {
- monitor_printf(mon, " auth: %s\n", info->auth);
+ if (!info2l) {
+ monitor_printf(mon, "None\n");
+ return;
}
- if (!info->has_clients || info->clients == NULL) {
- monitor_printf(mon, "Client: none\n");
- } else {
- for (client = info->clients; client; client = client->next) {
- monitor_printf(mon, "Client:\n");
- monitor_printf(mon, " address: %s:%s\n",
- client->value->host,
- client->value->service);
- monitor_printf(mon, " x509_dname: %s\n",
- client->value->x509_dname ?
- client->value->x509_dname : "none");
- monitor_printf(mon, " username: %s\n",
- client->value->has_sasl_username ?
- client->value->sasl_username : "none");
+ while (info2l) {
+ VncInfo2 *info = info2l->value;
+ monitor_printf(mon, "%s:\n", info->id);
+ hmp_info_vnc_servers(mon, info->server);
+ hmp_info_vnc_clients(mon, info->clients);
+ if (!info->server) {
+ /* The server entry displays its auth, we only
+ * need to display in the case of 'reverse' connections
+ * where there's no server.
+ */
+ hmp_info_vnc_authcrypt(mon, " ", info->auth,
+ info->has_vencrypt ? &info->vencrypt : NULL);
+ }
+ if (info->has_display) {
+ monitor_printf(mon, " Display: %s\n", info->display);
}
+ info2l = info2l->next;
}
-out:
- qapi_free_VncInfo(info);
+ qapi_free_VncInfo2List(info2l);
+
}
#ifdef CONFIG_SPICE
--
2.9.3
On Mon, 17 Jul 2017 at 10:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The QMP query-vnc interfaces have gained a lot more information that
> the HMP interfaces hasn't got yet. Update it.
>
> Note the output format has changed, but this is HMP so that's OK.
Hi; another "ancient change Coverity has only just noticed has
a problem" email :-) This is CID 1421932. It looks like any
"info vnc" will leak memory if there are any VNC servers to
display info about...
> void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> {
> - VncInfo *info;
> + VncInfo2List *info2l;
> Error *err = NULL;
> - VncClientInfoList *client;
>
> - info = qmp_query_vnc(&err);
> + info2l = qmp_query_vnc_servers(&err);
Here we get a list of VNC servers, which is allocated memory...
> if (err) {
> error_report_err(err);
> return;
> }
> -
> - if (!info->enabled) {
> - monitor_printf(mon, "Server: disabled\n");
> - goto out;
> - }
> -
> - monitor_printf(mon, "Server:\n");
> - if (info->has_host && info->has_service) {
> - monitor_printf(mon, " address: %s:%s\n", info->host, info->service);
> - }
> - if (info->has_auth) {
> - monitor_printf(mon, " auth: %s\n", info->auth);
> + if (!info2l) {
> + monitor_printf(mon, "None\n");
> + return;
> }
>
> - if (!info->has_clients || info->clients == NULL) {
> - monitor_printf(mon, "Client: none\n");
> - } else {
> - for (client = info->clients; client; client = client->next) {
> - monitor_printf(mon, "Client:\n");
> - monitor_printf(mon, " address: %s:%s\n",
> - client->value->host,
> - client->value->service);
> - monitor_printf(mon, " x509_dname: %s\n",
> - client->value->x509_dname ?
> - client->value->x509_dname : "none");
> - monitor_printf(mon, " username: %s\n",
> - client->value->has_sasl_username ?
> - client->value->sasl_username : "none");
> + while (info2l) {
> + VncInfo2 *info = info2l->value;
> + monitor_printf(mon, "%s:\n", info->id);
> + hmp_info_vnc_servers(mon, info->server);
> + hmp_info_vnc_clients(mon, info->clients);
> + if (!info->server) {
> + /* The server entry displays its auth, we only
> + * need to display in the case of 'reverse' connections
> + * where there's no server.
> + */
> + hmp_info_vnc_authcrypt(mon, " ", info->auth,
> + info->has_vencrypt ? &info->vencrypt : NULL);
> + }
> + if (info->has_display) {
> + monitor_printf(mon, " Display: %s\n", info->display);
> }
> + info2l = info2l->next;
...but the loop iteration here updates 'info2l' as it goes along...
> }
>
> -out:
> - qapi_free_VncInfo(info);
> + qapi_free_VncInfo2List(info2l);
...so here we end up passing NULL to qapi_free_VncInfo2List(),
which will do nothing, leaking the whole list.
Would somebody like to send a patch?
thanks
-- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Mon, 17 Jul 2017 at 10:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The QMP query-vnc interfaces have gained a lot more information that
> > the HMP interfaces hasn't got yet. Update it.
> >
> > Note the output format has changed, but this is HMP so that's OK.
>
> Hi; another "ancient change Coverity has only just noticed has
> a problem" email :-) This is CID 1421932. It looks like any
> "info vnc" will leak memory if there are any VNC servers to
> display info about...
>
> > void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> > {
> > - VncInfo *info;
> > + VncInfo2List *info2l;
> > Error *err = NULL;
> > - VncClientInfoList *client;
> >
> > - info = qmp_query_vnc(&err);
> > + info2l = qmp_query_vnc_servers(&err);
>
> Here we get a list of VNC servers, which is allocated memory...
>
> > if (err) {
> > error_report_err(err);
> > return;
> > }
> > -
> > - if (!info->enabled) {
> > - monitor_printf(mon, "Server: disabled\n");
> > - goto out;
> > - }
> > -
> > - monitor_printf(mon, "Server:\n");
> > - if (info->has_host && info->has_service) {
> > - monitor_printf(mon, " address: %s:%s\n", info->host, info->service);
> > - }
> > - if (info->has_auth) {
> > - monitor_printf(mon, " auth: %s\n", info->auth);
> > + if (!info2l) {
> > + monitor_printf(mon, "None\n");
> > + return;
> > }
> >
> > - if (!info->has_clients || info->clients == NULL) {
> > - monitor_printf(mon, "Client: none\n");
> > - } else {
> > - for (client = info->clients; client; client = client->next) {
> > - monitor_printf(mon, "Client:\n");
> > - monitor_printf(mon, " address: %s:%s\n",
> > - client->value->host,
> > - client->value->service);
> > - monitor_printf(mon, " x509_dname: %s\n",
> > - client->value->x509_dname ?
> > - client->value->x509_dname : "none");
> > - monitor_printf(mon, " username: %s\n",
> > - client->value->has_sasl_username ?
> > - client->value->sasl_username : "none");
> > + while (info2l) {
> > + VncInfo2 *info = info2l->value;
> > + monitor_printf(mon, "%s:\n", info->id);
> > + hmp_info_vnc_servers(mon, info->server);
> > + hmp_info_vnc_clients(mon, info->clients);
> > + if (!info->server) {
> > + /* The server entry displays its auth, we only
> > + * need to display in the case of 'reverse' connections
> > + * where there's no server.
> > + */
> > + hmp_info_vnc_authcrypt(mon, " ", info->auth,
> > + info->has_vencrypt ? &info->vencrypt : NULL);
> > + }
> > + if (info->has_display) {
> > + monitor_printf(mon, " Display: %s\n", info->display);
> > }
> > + info2l = info2l->next;
>
> ...but the loop iteration here updates 'info2l' as it goes along...
>
> > }
> >
> > -out:
> > - qapi_free_VncInfo(info);
> > + qapi_free_VncInfo2List(info2l);
>
> ...so here we end up passing NULL to qapi_free_VncInfo2List(),
> which will do nothing, leaking the whole list.
>
> Would somebody like to send a patch?
Oops, yes I can look at that; I guess something along the lines of an
info2l_orig and free that at the end.
Dave
> thanks
> -- PMM
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2026 Red Hat, Inc.