From nobody Mon Feb 9 06:00:35 2026 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 1488902118578412.61550344476063; Tue, 7 Mar 2017 07:55:18 -0800 (PST) Received: from localhost ([::1]:51322 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1clHSG-0005Hc-6l for importer@patchew.org; Tue, 07 Mar 2017 10:55:16 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1clHEs-0002Mq-42 for qemu-devel@nongnu.org; Tue, 07 Mar 2017 10:41:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1clHEq-0002px-Qz for qemu-devel@nongnu.org; Tue, 07 Mar 2017 10:41:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57852) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1clHEo-0002nE-2c; Tue, 07 Mar 2017 10:41:22 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2B9BCC04B951; Tue, 7 Mar 2017 15:41:22 +0000 (UTC) Received: from noname.str.redhat.com (dhcp-192-197.str.redhat.com [10.33.192.197]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v27FeuUi032123; Tue, 7 Mar 2017 10:41:21 -0500 From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 7 Mar 2017 16:40:39 +0100 Message-Id: <1488901251-16214-16-git-send-email-kwolf@redhat.com> In-Reply-To: <1488901251-16214-1-git-send-email-kwolf@redhat.com> References: <1488901251-16214-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 07 Mar 2017 15:41:22 +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 15/27] sheepdog: Fix snapshot ID parsing in _open(), _create, _goto() 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: kwolf@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: Markus Armbruster sd_parse_uri() and sd_snapshot_goto() screw up error checking after strtoul(), and truncate long tag names silently. Fix by replacing those parts by new sd_parse_snapid_or_tag(), which checks more carefully. sd_snapshot_delete() also parses snapshot IDs, but is currently too broken for me to touch. Mark TODO. Two calls of strtol() without error checking remain in parse_redundancy(). Mark them FIXME. More silent truncation of configuration strings remains elsewhere. Not marked. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/sheepdog.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++------= ---- 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 187bcd8..d3d3731 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -914,6 +914,49 @@ static int get_sheep_fd(BDRVSheepdogState *s, Error **= errp) return fd; } =20 +/* + * Parse numeric snapshot ID in @str + * If @str can't be parsed as number, return false. + * Else, if the number is zero or too large, set *@snapid to zero and + * return true. + * Else, set *@snapid to the number and return true. + */ +static bool sd_parse_snapid(const char *str, uint32_t *snapid) +{ + unsigned long ul; + int ret; + + ret =3D qemu_strtoul(str, NULL, 10, &ul); + if (ret =3D=3D -ERANGE) { + ul =3D ret =3D 0; + } + if (ret) { + return false; + } + if (ul > UINT32_MAX) { + ul =3D 0; + } + + *snapid =3D ul; + return true; +} + +static bool sd_parse_snapid_or_tag(const char *str, + uint32_t *snapid, char tag[]) +{ + if (!sd_parse_snapid(str, snapid)) { + *snapid =3D 0; + if (g_strlcpy(tag, str, SD_MAX_VDI_TAG_LEN) >=3D SD_MAX_VDI_TAG_LE= N) { + return false; + } + } else if (!*snapid) { + return false; + } else { + tag[0] =3D 0; + } + return true; +} + static int sd_parse_uri(BDRVSheepdogState *s, const char *filename, char *vdi, uint32_t *snapid, char *tag) { @@ -965,9 +1008,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const ch= ar *filename, =20 /* snapshot tag */ if (uri->fragment) { - *snapid =3D strtoul(uri->fragment, NULL, 10); - if (*snapid =3D=3D 0) { - pstrcpy(tag, SD_MAX_VDI_TAG_LEN, uri->fragment); + if (!sd_parse_snapid_or_tag(uri->fragment, snapid, tag)) { + ret =3D -EINVAL; + goto out; } } else { *snapid =3D CURRENT_VDI_ID; /* search current vdi */ @@ -1685,6 +1728,7 @@ static int parse_redundancy(BDRVSheepdogState *s, con= st char *opt) } =20 copy =3D strtol(n1, NULL, 10); + /* FIXME fix error checking by switching to qemu_strtol() */ if (copy > SD_MAX_COPIES || copy < 1) { return -EINVAL; } @@ -1699,6 +1743,7 @@ static int parse_redundancy(BDRVSheepdogState *s, con= st char *opt) } =20 parity =3D strtol(n2, NULL, 10); + /* FIXME fix error checking by switching to qemu_strtol() */ if (parity >=3D SD_EC_MAX_STRIP || parity < 1) { return -EINVAL; } @@ -2365,19 +2410,16 @@ static int sd_snapshot_goto(BlockDriverState *bs, c= onst char *snapshot_id) BDRVSheepdogState *old_s; char tag[SD_MAX_VDI_TAG_LEN]; uint32_t snapid =3D 0; - int ret =3D 0; + int ret; + + if (!sd_parse_snapid_or_tag(snapshot_id, &snapid, tag)) { + return -EINVAL; + } =20 old_s =3D g_new(BDRVSheepdogState, 1); =20 memcpy(old_s, s, sizeof(BDRVSheepdogState)); =20 - snapid =3D strtoul(snapshot_id, NULL, 10); - if (snapid) { - tag[0] =3D 0; - } else { - pstrcpy(tag, sizeof(tag), snapshot_id); - } - ret =3D reload_inode(s, snapid, tag); if (ret) { goto out; @@ -2483,6 +2525,7 @@ static int sd_snapshot_delete(BlockDriverState *bs, memset(buf, 0, sizeof(buf)); memset(snap_tag, 0, sizeof(snap_tag)); pstrcpy(buf, SD_MAX_VDI_LEN, s->name); + /* TODO Use sd_parse_snapid() once this mess is cleaned up */ ret =3D qemu_strtoul(snapshot_id, NULL, 10, &snap_id); if (ret || snap_id > UINT32_MAX) { /* @@ -2499,6 +2542,7 @@ static int sd_snapshot_delete(BlockDriverState *bs, hdr.snapid =3D (uint32_t) snap_id; } else { /* FIXME I suspect we should use @name here */ + /* FIXME don't truncate silently */ pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id); pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag); } --=20 1.8.3.1