From 8a6a80896d6af03b8ee0c17cdf37219eca2588a7 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 15 Aug 2016 15:29:23 +0200 Subject: block/ssh: Use QemuOpts for runtime options Using QemuOpts will prevent qemu from crashing if the input options have not been validated (which is the case when they are specified on the command line or in a json: filename) and some have the wrong type. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/ssh.c | 79 ++++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 24 deletions(-) (limited to 'block') diff --git a/block/ssh.c b/block/ssh.c index bcbb0e4223..5ce12b633a 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -508,36 +508,73 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp) return ret; } +static QemuOptsList ssh_runtime_opts = { + .name = "ssh", + .head = QTAILQ_HEAD_INITIALIZER(ssh_runtime_opts.head), + .desc = { + { + .name = "host", + .type = QEMU_OPT_STRING, + .help = "Host to connect to", + }, + { + .name = "port", + .type = QEMU_OPT_NUMBER, + .help = "Port to connect to", + }, + { + .name = "path", + .type = QEMU_OPT_STRING, + .help = "Path of the image on the host", + }, + { + .name = "user", + .type = QEMU_OPT_STRING, + .help = "User as which to connect", + }, + { + .name = "host_key_check", + .type = QEMU_OPT_STRING, + .help = "Defines how and what to check the host key against", + }, + }, +}; + static int connect_to_ssh(BDRVSSHState *s, QDict *options, int ssh_flags, int creat_mode, Error **errp) { int r, ret; + QemuOpts *opts = NULL; + Error *local_err = NULL; const char *host, *user, *path, *host_key_check; int port; - if (!qdict_haskey(options, "host")) { + opts = qemu_opts_create(&ssh_runtime_opts, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err) { ret = -EINVAL; - error_setg(errp, "No hostname was specified"); + error_propagate(errp, local_err); goto err; } - host = qdict_get_str(options, "host"); - if (qdict_haskey(options, "port")) { - port = qdict_get_int(options, "port"); - } else { - port = 22; + host = qemu_opt_get(opts, "host"); + if (!host) { + ret = -EINVAL; + error_setg(errp, "No hostname was specified"); + goto err; } - if (!qdict_haskey(options, "path")) { + port = qemu_opt_get_number(opts, "port", 22); + + path = qemu_opt_get(opts, "path"); + if (!path) { ret = -EINVAL; error_setg(errp, "No path was specified"); goto err; } - path = qdict_get_str(options, "path"); - if (qdict_haskey(options, "user")) { - user = qdict_get_str(options, "user"); - } else { + user = qemu_opt_get(opts, "user"); + if (!user) { user = g_get_user_name(); if (!user) { error_setg_errno(errp, errno, "Can't get user name"); @@ -546,9 +583,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, } } - if (qdict_haskey(options, "host_key_check")) { - host_key_check = qdict_get_str(options, "host_key_check"); - } else { + host_key_check = qemu_opt_get(opts, "host_key_check"); + if (!host_key_check) { host_key_check = "yes"; } @@ -612,21 +648,14 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, goto err; } + qemu_opts_del(opts); + r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs); if (r < 0) { sftp_error_setg(errp, s, "failed to read file attributes"); return -EINVAL; } - /* Delete the options we've used; any not deleted will cause the - * block layer to give an error about unused options. - */ - qdict_del(options, "host"); - qdict_del(options, "port"); - qdict_del(options, "user"); - qdict_del(options, "path"); - qdict_del(options, "host_key_check"); - return 0; err: @@ -646,6 +675,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, } s->session = NULL; + qemu_opts_del(opts); + return ret; } -- cgit v1.2.1 From 7ccc44fd7d1dfa62c4d6f3a680df809d6e7068ce Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 15 Aug 2016 15:29:24 +0200 Subject: block/nbd: Use QemuOpts for runtime options Using QemuOpts will prevent qemu from crashing if the input options have not been validated (which is the case when they are specified on the command line or in a json: filename) and some have the wrong type. Signed-off-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/nbd.c | 74 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 20 deletions(-) (limited to 'block') diff --git a/block/nbd.c b/block/nbd.c index 8d57220f18..60096daba3 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -188,13 +188,13 @@ out: g_free(file); } -static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export, +static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export, Error **errp) { SocketAddress *saddr; - if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) { - if (qdict_haskey(options, "path")) { + if (!qemu_opt_get(opts, "path") == !qemu_opt_get(opts, "host")) { + if (qemu_opt_get(opts, "path")) { error_setg(errp, "path and host may not be used at the same time."); } else { error_setg(errp, "one of path and host must be specified."); @@ -204,32 +204,25 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export, saddr = g_new0(SocketAddress, 1); - if (qdict_haskey(options, "path")) { + if (qemu_opt_get(opts, "path")) { UnixSocketAddress *q_unix; saddr->type = SOCKET_ADDRESS_KIND_UNIX; q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1); - q_unix->path = g_strdup(qdict_get_str(options, "path")); - qdict_del(options, "path"); + q_unix->path = g_strdup(qemu_opt_get(opts, "path")); } else { InetSocketAddress *inet; saddr->type = SOCKET_ADDRESS_KIND_INET; inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1); - inet->host = g_strdup(qdict_get_str(options, "host")); - if (!qdict_get_try_str(options, "port")) { + inet->host = g_strdup(qemu_opt_get(opts, "host")); + inet->port = g_strdup(qemu_opt_get(opts, "port")); + if (!inet->port) { inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT); - } else { - inet->port = g_strdup(qdict_get_str(options, "port")); } - qdict_del(options, "host"); - qdict_del(options, "port"); } s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX; - *export = g_strdup(qdict_get_try_str(options, "export")); - if (*export) { - qdict_del(options, "export"); - } + *export = g_strdup(qemu_opt_get(opts, "export")); return saddr; } @@ -292,27 +285,67 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) } +static QemuOptsList nbd_runtime_opts = { + .name = "nbd", + .head = QTAILQ_HEAD_INITIALIZER(nbd_runtime_opts.head), + .desc = { + { + .name = "host", + .type = QEMU_OPT_STRING, + .help = "TCP host to connect to", + }, + { + .name = "port", + .type = QEMU_OPT_STRING, + .help = "TCP port to connect to", + }, + { + .name = "path", + .type = QEMU_OPT_STRING, + .help = "Unix socket path to connect to", + }, + { + .name = "export", + .type = QEMU_OPT_STRING, + .help = "Name of the NBD export to open", + }, + { + .name = "tls-creds", + .type = QEMU_OPT_STRING, + .help = "ID of the TLS credentials to use", + }, + }, +}; + static int nbd_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVNBDState *s = bs->opaque; + QemuOpts *opts = NULL; + Error *local_err = NULL; char *export = NULL; QIOChannelSocket *sioc = NULL; - SocketAddress *saddr; + SocketAddress *saddr = NULL; const char *tlscredsid; QCryptoTLSCreds *tlscreds = NULL; const char *hostname = NULL; int ret = -EINVAL; + opts = qemu_opts_create(&nbd_runtime_opts, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto error; + } + /* Pop the config into our state object. Exit if invalid. */ - saddr = nbd_config(s, options, &export, errp); + saddr = nbd_config(s, opts, &export, errp); if (!saddr) { goto error; } - tlscredsid = g_strdup(qdict_get_try_str(options, "tls-creds")); + tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds")); if (tlscredsid) { - qdict_del(options, "tls-creds"); tlscreds = nbd_get_tls_creds(tlscredsid, errp); if (!tlscreds) { goto error; @@ -346,6 +379,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, } qapi_free_SocketAddress(saddr); g_free(export); + qemu_opts_del(opts); return ret; } -- cgit v1.2.1 From 036990d72b00c920fc10158c14259fb59dbe61a1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 15 Aug 2016 15:29:25 +0200 Subject: block/blkdebug: Store config filename Store the configuration file's filename so it can later be used in bdrv_refresh_filename() without having to directly access the options QDict which may contain a value of a non-string type. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/blkdebug.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/blkdebug.c b/block/blkdebug.c index fb29283f80..d5db166815 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -39,6 +39,9 @@ typedef struct BDRVBlkdebugState { int new_state; int align; + /* For blkdebug_refresh_filename() */ + char *config_file; + QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX]; QSIMPLEQ_HEAD(, BlkdebugRule) active_rules; QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs; @@ -351,7 +354,6 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, BDRVBlkdebugState *s = bs->opaque; QemuOpts *opts; Error *local_err = NULL; - const char *config; uint64_t align; int ret; @@ -364,8 +366,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, } /* Read rules from config file or command line options */ - config = qemu_opt_get(opts, "config"); - ret = read_config(s, config, options, errp); + s->config_file = g_strdup(qemu_opt_get(opts, "config")); + ret = read_config(s, s->config_file, options, errp); if (ret) { goto out; } @@ -398,6 +400,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, fail_unref: bdrv_unref_child(bs, bs->file); out: + if (ret < 0) { + g_free(s->config_file); + } qemu_opts_del(opts); return ret; } @@ -515,6 +520,8 @@ static void blkdebug_close(BlockDriverState *bs) remove_rule(rule); } } + + g_free(s->config_file); } static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule) @@ -679,6 +686,7 @@ static int blkdebug_truncate(BlockDriverState *bs, int64_t offset) static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options) { + BDRVBlkdebugState *s = bs->opaque; QDict *opts; const QDictEntry *e; bool force_json = false; @@ -700,8 +708,7 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options) if (!force_json && bs->file->bs->exact_filename[0]) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "blkdebug:%s:%s", - qdict_get_try_str(options, "config") ?: "", + "blkdebug:%s:%s", s->config_file ?: "", bs->file->bs->exact_filename); } -- cgit v1.2.1 From 03504d05f0618d1773d2335138fd7b8b18f32afb Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 15 Aug 2016 15:29:26 +0200 Subject: block/nbd: Store runtime option values Store the runtime option values in the BDRVNBDState so they can later be used in nbd_refresh_filename() without having to directly access the options QDict which may contain values of non-string types. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/nbd.c | 105 +++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 61 insertions(+), 44 deletions(-) (limited to 'block') diff --git a/block/nbd.c b/block/nbd.c index 60096daba3..6bc06d6198 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -42,6 +42,9 @@ typedef struct BDRVNBDState { NbdClientSession client; + + /* For nbd_refresh_filename() */ + char *path, *host, *port, *export, *tlscredsid; } BDRVNBDState; static int nbd_parse_uri(const char *filename, QDict *options) @@ -188,13 +191,15 @@ out: g_free(file); } -static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export, - Error **errp) +static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp) { SocketAddress *saddr; - if (!qemu_opt_get(opts, "path") == !qemu_opt_get(opts, "host")) { - if (qemu_opt_get(opts, "path")) { + s->path = g_strdup(qemu_opt_get(opts, "path")); + s->host = g_strdup(qemu_opt_get(opts, "host")); + + if (!s->path == !s->host) { + if (s->path) { error_setg(errp, "path and host may not be used at the same time."); } else { error_setg(errp, "one of path and host must be specified."); @@ -204,17 +209,20 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export, saddr = g_new0(SocketAddress, 1); - if (qemu_opt_get(opts, "path")) { + if (s->path) { UnixSocketAddress *q_unix; saddr->type = SOCKET_ADDRESS_KIND_UNIX; q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1); - q_unix->path = g_strdup(qemu_opt_get(opts, "path")); + q_unix->path = g_strdup(s->path); } else { InetSocketAddress *inet; + + s->port = g_strdup(qemu_opt_get(opts, "port")); + saddr->type = SOCKET_ADDRESS_KIND_INET; inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1); - inet->host = g_strdup(qemu_opt_get(opts, "host")); - inet->port = g_strdup(qemu_opt_get(opts, "port")); + inet->host = g_strdup(s->host); + inet->port = g_strdup(s->port); if (!inet->port) { inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT); } @@ -222,7 +230,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export, s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX; - *export = g_strdup(qemu_opt_get(opts, "export")); + s->export = g_strdup(qemu_opt_get(opts, "export")); return saddr; } @@ -323,10 +331,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, BDRVNBDState *s = bs->opaque; QemuOpts *opts = NULL; Error *local_err = NULL; - char *export = NULL; QIOChannelSocket *sioc = NULL; SocketAddress *saddr = NULL; - const char *tlscredsid; QCryptoTLSCreds *tlscreds = NULL; const char *hostname = NULL; int ret = -EINVAL; @@ -339,14 +345,14 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, } /* Pop the config into our state object. Exit if invalid. */ - saddr = nbd_config(s, opts, &export, errp); + saddr = nbd_config(s, opts, errp); if (!saddr) { goto error; } - tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds")); - if (tlscredsid) { - tlscreds = nbd_get_tls_creds(tlscredsid, errp); + s->tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds")); + if (s->tlscredsid) { + tlscreds = nbd_get_tls_creds(s->tlscredsid, errp); if (!tlscreds) { goto error; } @@ -368,7 +374,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, } /* NBD handshake */ - ret = nbd_client_init(bs, sioc, export, + ret = nbd_client_init(bs, sioc, s->export, tlscreds, hostname, errp); error: if (sioc) { @@ -377,8 +383,14 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, if (tlscreds) { object_unref(OBJECT(tlscreds)); } + if (ret < 0) { + g_free(s->path); + g_free(s->host); + g_free(s->port); + g_free(s->export); + g_free(s->tlscredsid); + } qapi_free_SocketAddress(saddr); - g_free(export); qemu_opts_del(opts); return ret; } @@ -396,7 +408,15 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp) static void nbd_close(BlockDriverState *bs) { + BDRVNBDState *s = bs->opaque; + nbd_client_close(bs); + + g_free(s->path); + g_free(s->host); + g_free(s->port); + g_free(s->export); + g_free(s->tlscredsid); } static int64_t nbd_getlength(BlockDriverState *bs) @@ -419,48 +439,45 @@ static void nbd_attach_aio_context(BlockDriverState *bs, static void nbd_refresh_filename(BlockDriverState *bs, QDict *options) { + BDRVNBDState *s = bs->opaque; QDict *opts = qdict_new(); - const char *path = qdict_get_try_str(options, "path"); - const char *host = qdict_get_try_str(options, "host"); - const char *port = qdict_get_try_str(options, "port"); - const char *export = qdict_get_try_str(options, "export"); - const char *tlscreds = qdict_get_try_str(options, "tls-creds"); qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd"))); - if (path && export) { + if (s->path && s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd+unix:///%s?socket=%s", export, path); - } else if (path && !export) { + "nbd+unix:///%s?socket=%s", s->export, s->path); + } else if (s->path && !s->export) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd+unix://?socket=%s", path); - } else if (!path && export && port) { + "nbd+unix://?socket=%s", s->path); + } else if (!s->path && s->export && s->port) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s/%s", host, port, export); - } else if (!path && export && !port) { + "nbd://%s:%s/%s", s->host, s->port, s->export); + } else if (!s->path && s->export && !s->port) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s/%s", host, export); - } else if (!path && !export && port) { + "nbd://%s/%s", s->host, s->export); + } else if (!s->path && !s->export && s->port) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s:%s", host, port); - } else if (!path && !export && !port) { + "nbd://%s:%s", s->host, s->port); + } else if (!s->path && !s->export && !s->port) { snprintf(bs->exact_filename, sizeof(bs->exact_filename), - "nbd://%s", host); + "nbd://%s", s->host); } - if (path) { - qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path))); - } else if (port) { - qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host))); - qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port))); + if (s->path) { + qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path))); + } else if (s->port) { + qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host))); + qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(s->port))); } else { - qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host))); + qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host))); } - if (export) { - qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export))); + if (s->export) { + qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export))); } - if (tlscreds) { - qdict_put_obj(opts, "tls-creds", QOBJECT(qstring_from_str(tlscreds))); + if (s->tlscredsid) { + qdict_put_obj(opts, "tls-creds", + QOBJECT(qstring_from_str(s->tlscredsid))); } bs->full_open_options = opts; -- cgit v1.2.1