From nobody Thu Dec 18 17:53:29 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1491010023135515.5346194799598; Fri, 31 Mar 2017 18:27:03 -0700 (PDT) Received: from localhost ([::1]:43515 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cu7oj-0002V4-Pu for importer@patchew.org; Fri, 31 Mar 2017 21:27:01 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cu7n7-0001Tx-UD for qemu-devel@nongnu.org; Fri, 31 Mar 2017 21:25:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cu7n6-0005RX-Ld for qemu-devel@nongnu.org; Fri, 31 Mar 2017 21:25:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34118) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cu7n2-0005LI-AL; Fri, 31 Mar 2017 21:25:16 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD612C05973B; Sat, 1 Apr 2017 01:25:10 +0000 (UTC) Received: from localhost (ovpn-117-90.phx2.redhat.com [10.3.117.90]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8A9065C54F; Sat, 1 Apr 2017 01:25:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DD612C05973B Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jcody@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com DD612C05973B From: Jeff Cody To: qemu-block@nongnu.org Date: Fri, 31 Mar 2017 21:25:06 -0400 Message-Id: <20170401012508.24665-2-jcody@redhat.com> In-Reply-To: <20170401012508.24665-1-jcody@redhat.com> References: <20170401012508.24665-1-jcody@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Sat, 01 Apr 2017 01:25:11 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL for-2.9 1/3] rbd: Fix regression in legacy key/values containing escaped : X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, jcody@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Eric Blake Commit c7cacb3 accidentally broke legacy key-value parsing through pseudo-filename parsing of -drive file=3Drbd://..., for any key that contains an escaped ':'. Such a key is surprisingly common, thanks to mon_host specifying a 'host:port' string. The break happens because passing things from QDict through QemuOpts back to another QDict requires that we pack our parsed key/value pairs into a string, and then reparse that string, but the intermediate string that we created ("key1=3Dvalue1:key2=3Dvalue2") lost the \: escaping that was present in the original, so that we could no longer see which : were used as separators vs. those used as part of the original input. Fix it by collecting the key/value pairs through a QList, and sending that list on a round trip through a JSON QString (as in '["key1","value1","key2","value2"]') on its way through QemuOpts, rather than hand-rolling our own string. Since the string is only handled internally, this was faster than creating a full-blown struct of '[{"key1":"value1"},{"key2":"value2"}]', and safer at guaranteeing order compared to '{"key1":"value1","key2":"value2"}'. It would be nicer if we didn't have to round-trip through QemuOpts in the first place, but that's a much bigger task for later. Reproducer: ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ -drive 'file=3Drbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70'\ ':id=3Dcompute:key=3DAQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg=3D=3D'\ ':auth_supported=3Dcephx\;none:mon_host=3D192.168.1.2\:6789'\ ',format=3Draw,if=3Dnone,id=3Ddrive-virtio-disk0,'\ 'serial=3Dea141b5c-cdb3-4765-910d-e7008b209a70,cache=3Dwriteback' Even without an RBD setup, this serves a test of whether we get the incorrect parser error of: qemu-system-x86_64: -drive file=3Drbd:...cache=3Dwriteback: conf option 678= 9 has no value or the correct behavior of hanging while trying to connect to the requested mon_host of 192.168.1.2:6789. Reported-by: Alexandru Avadanii Signed-off-by: Eric Blake Reviewed-by: Jeff Cody Reviewed-by: Max Reitz Message-id: 20170331152730.12514-1-eblake@redhat.com Signed-off-by: Jeff Cody --- block/rbd.c | 83 +++++++++++++++++++++++++++++++--------------------------= ---- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index 498322b..fbdb131 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -20,6 +20,7 @@ #include "crypto/secret.h" #include "qemu/cutils.h" #include "qapi/qmp/qstring.h" +#include "qapi/qmp/qjson.h" =20 /* * When specifying the image filename use: @@ -135,18 +136,16 @@ static void qemu_rbd_parse_filename(const char *filen= ame, QDict *options, Error **errp) { const char *start; - char *p, *buf, *keypairs; + char *p, *buf; + QList *keypairs =3D NULL; char *found_str; - size_t max_keypair_size; =20 if (!strstart(filename, "rbd:", &start)) { error_setg(errp, "File name must start with 'rbd:'"); return; } =20 - max_keypair_size =3D strlen(start) + 1; buf =3D g_strdup(start); - keypairs =3D g_malloc0(max_keypair_size); p =3D buf; =20 found_str =3D qemu_rbd_next_tok(p, '/', &p); @@ -194,33 +193,30 @@ static void qemu_rbd_parse_filename(const char *filen= ame, QDict *options, } else if (!strcmp(name, "id")) { qdict_put(options, "user" , qstring_from_str(value)); } else { - /* FIXME: This is pretty ugly, and not the right way to do thi= s. - * These should be contained in a structure, and then - * passed explicitly as individual key/value pairs to - * rados. Consider this legacy code that needs to be - * updated. */ - char *tmp =3D g_malloc0(max_keypair_size); - /* only use a delimiter if it is not the first keypair found */ - /* These are sets of unknown key/value pairs we'll pass along - * to ceph */ - if (keypairs[0]) { - snprintf(tmp, max_keypair_size, ":%s=3D%s", name, value); - pstrcat(keypairs, max_keypair_size, tmp); - } else { - snprintf(keypairs, max_keypair_size, "%s=3D%s", name, valu= e); + /* + * We pass these internally to qemu_rbd_set_keypairs(), so + * we can get away with the simpler list of [ "key1", + * "value1", "key2", "value2" ] rather than a raw dict + * { "key1": "value1", "key2": "value2" } where we can't + * guarantee order, or even a more correct but complex + * [ { "key1": "value1" }, { "key2": "value2" } ] + */ + if (!keypairs) { + keypairs =3D qlist_new(); } - g_free(tmp); + qlist_append(keypairs, qstring_from_str(name)); + qlist_append(keypairs, qstring_from_str(value)); } } =20 - if (keypairs[0]) { - qdict_put(options, "=3Dkeyvalue-pairs", qstring_from_str(keypairs)= ); + if (keypairs) { + qdict_put(options, "=3Dkeyvalue-pairs", + qobject_to_json(QOBJECT(keypairs))); } =20 - done: g_free(buf); - g_free(keypairs); + QDECREF(keypairs); return; } =20 @@ -244,36 +240,41 @@ static int qemu_rbd_set_auth(rados_t cluster, const c= har *secretid, return 0; } =20 -static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs, +static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_jso= n, Error **errp) { - char *p, *buf; - char *name; - char *value; + QList *keypairs; + QString *name; + QString *value; + const char *key; + size_t remaining; int ret =3D 0; =20 - buf =3D g_strdup(keypairs); - p =3D buf; + if (!keypairs_json) { + return ret; + } + keypairs =3D qobject_to_qlist(qobject_from_json(keypairs_json, + &error_abort)); + remaining =3D qlist_size(keypairs) / 2; + assert(remaining); =20 - while (p) { - name =3D qemu_rbd_next_tok(p, '=3D', &p); - if (!p) { - error_setg(errp, "conf option %s has no value", name); - ret =3D -EINVAL; - break; - } + while (remaining--) { + name =3D qobject_to_qstring(qlist_pop(keypairs)); + value =3D qobject_to_qstring(qlist_pop(keypairs)); + assert(name && value); + key =3D qstring_get_str(name); =20 - value =3D qemu_rbd_next_tok(p, ':', &p); - - ret =3D rados_conf_set(cluster, name, value); + ret =3D rados_conf_set(cluster, key, qstring_get_str(value)); + QDECREF(name); + QDECREF(value); if (ret < 0) { - error_setg_errno(errp, -ret, "invalid conf option %s", name); + error_setg_errno(errp, -ret, "invalid conf option %s", key); ret =3D -EINVAL; break; } } =20 - g_free(buf); + QDECREF(keypairs); return ret; } =20 --=20 2.9.3