From nobody Thu Oct 30 15:33:08 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.zohomail.com; dkim=fail; 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1525818566731701.1676542595357; Tue, 8 May 2018 15:29:26 -0700 (PDT) Received: from localhost ([::1]:53587 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGB6i-00080G-Sk for importer@patchew.org; Tue, 08 May 2018 18:29:16 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGAt7-00036o-4z for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fGAt3-0001Ry-Su for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:13 -0400 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]:56240) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fGAt3-0001Qt-IM for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:09 -0400 Received: by mail-wm0-x229.google.com with SMTP id a8so21229655wmg.5 for ; Tue, 08 May 2018 15:15:09 -0700 (PDT) Received: from 640k.lan (dynamic-adsl-78-12-189-60.clienti.tiscali.it. [78.12.189.60]) by smtp.gmail.com with ESMTPSA id c15-v6sm14020129edr.78.2018.05.08.15.15.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 May 2018 15:15:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=9R2MPoIIv2e28QzJ0ZP2kesyCPah6gyyJ8mImJ4G2lE=; b=UlVEiWHNxAaaBFFmuYECir9rG6v0NW0pgpfx4+5OGStKuOD4z6Qn7ViI1eyrMOY37M +qp+9q1Gu6qCHq10zaale/cRQxtcdiTSdqVrpERrtSyXEgVumPwm7xztDVf+QQB3GqaB +hSmq4gmweWASSxk9Cmbb7moZ5i2zSr4LRkOw6NktbcKnk5Lf/MebJlfLEB2XMkVDmDc CKFfEeRJeYCQ6C7L5Xpq7ECjy2haYE5ORFgIOCLYDhTzG5oQMA2D9R7u2MFLX3RCVy42 7kEkqXy5RmdEe5qKFXAkoo10ryRrK1MR8zCmZ6m6UuHIT3WbwWvGXgFgC4o60qamp/Nu XWTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references:mime-version:content-transfer-encoding; bh=9R2MPoIIv2e28QzJ0ZP2kesyCPah6gyyJ8mImJ4G2lE=; b=sp56vnvq9Fsb6wLDO9J80eftsGJ+EqubXUMwxm9GyIV2cSCc0l8OvZihRLnx4qPwT9 YBN8yQGFYcDUiKrpPO1ipghVCswtBwqEVNDUN4zMLy+sa1RHDYhCW97aM/a7eUENbfKV oMUzMJh3VzllWq3jjf2L2f0llM8X3vzAuMqMh5nGW15fzF63gQ+C/AMEIuPW8IHgbL52 6qXAxHWDW3K4kKqTHya6id4ADnqPtu/nsCbpfcJrjk1sLhim/7upQS1g0mYVVjBpXDeI h3+nqG/XyaQdDBp+aGoQDMZ+Kx8NneRUmFKvbf+scBQKB88xqxP9Eziwus3MSw0ttP/m xX/g== X-Gm-Message-State: ALQs6tCemeWCdhG+3C4NQoBgnsM+/JnkYwdgR3o/Nq0mEXDSIllGOxrD 5A9DFiEY/l+EOY7gJGERUpfb/rj1 X-Google-Smtp-Source: AB8JxZrZBSfqDX4nwl9uHj92kgEmsq79hawyRybQqV0mMOFcSV7OHXkZOdViKkVAIOGan4efCbGhwg== X-Received: by 2002:a50:b763:: with SMTP id g90-v6mr14187612ede.129.1525817708065; Tue, 08 May 2018 15:15:08 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 9 May 2018 00:14:32 +0200 Message-Id: <1525817687-34620-16-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1525817687-34620-1-git-send-email-pbonzini@redhat.com> References: <1525817687-34620-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:400c:c09::229 Subject: [Qemu-devel] [PULL 15/30] opts: don't silently truncate long option values 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: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 From: Daniel P. Berrang=C3=A9 The existing QemuOpts parsing code uses a fixed size 1024 byte buffer for storing the option values. If a value exceeded this size it was silently truncated and no error reported to the user. Long option values is not a common scenario, but it is conceivable that they will happen. eg if the user has a very deeply nested filesystem it would be possible to come up with a disk path that was > 1024 bytes. Most of the time if such data was silently truncated, the user would get an error about opening a non-existant disk. If they're unlucky though, QEMU might use a completely different disk image from another VM, which could be considered a security issue. Another example program was in using the -smbios command line arg with very large data blobs. In this case the silent truncation will be providing semantically incorrect data to the guest OS for SMBIOS tables. If the operating system didn't limit the user's argv when spawning QEMU, the code should honour whatever length arguments were given without imposing its own length restrictions. This patch thus changes the code to use a heap allocated buffer for storing the values during parsing, lifting the arbitrary length restriction. Signed-off-by: Daniel P. Berrang=C3=83=C2=A9 Message-Id: <20180416111743.8473-4-berrange@redhat.com> Signed-off-by: Paolo Bonzini Signed-off-by: Daniel P. Berrang=C3=A9 --- hw/i386/multiboot.c | 33 +++++++++------ include/qemu/option.h | 2 +- util/qemu-option.c | 111 +++++++++++++++++++++++++++-------------------= ---- 3 files changed, 81 insertions(+), 65 deletions(-) diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c index 5bc0a2c..7a2953e 100644 --- a/hw/i386/multiboot.c +++ b/hw/i386/multiboot.c @@ -291,12 +291,16 @@ int load_multiboot(FWCfgState *fw_cfg, cmdline_len =3D strlen(kernel_filename) + 1; cmdline_len +=3D strlen(kernel_cmdline) + 1; if (initrd_filename) { - const char *r =3D initrd_filename; + const char *r =3D get_opt_value(initrd_filename, NULL); cmdline_len +=3D strlen(r) + 1; mbs.mb_mods_avail =3D 1; - while (*(r =3D get_opt_value(NULL, 0, r))) { - mbs.mb_mods_avail++; - r++; + while (1) { + mbs.mb_mods_avail++; + r =3D get_opt_value(r, NULL); + if (!*r) { + break; + } + r++; } } =20 @@ -313,7 +317,8 @@ int load_multiboot(FWCfgState *fw_cfg, =20 if (initrd_filename) { const char *next_initrd; - char not_last, tmpbuf[strlen(initrd_filename) + 1]; + char not_last; + char *one_file =3D NULL; =20 mbs.offset_mods =3D mbs.mb_buf_size; =20 @@ -322,24 +327,26 @@ int load_multiboot(FWCfgState *fw_cfg, int mb_mod_length; uint32_t offs =3D mbs.mb_buf_size; =20 - next_initrd =3D get_opt_value(tmpbuf, sizeof(tmpbuf), initrd_f= ilename); + next_initrd =3D get_opt_value(initrd_filename, &one_file); not_last =3D *next_initrd; /* if a space comes after the module filename, treat everything after that as parameters */ - hwaddr c =3D mb_add_cmdline(&mbs, tmpbuf); - if ((next_space =3D strchr(tmpbuf, ' '))) + hwaddr c =3D mb_add_cmdline(&mbs, one_file); + next_space =3D strchr(one_file, ' '); + if (next_space) { *next_space =3D '\0'; - mb_debug("multiboot loading module: %s", tmpbuf); - mb_mod_length =3D get_image_size(tmpbuf); + } + mb_debug("multiboot loading module: %s", one_file); + mb_mod_length =3D get_image_size(one_file); if (mb_mod_length < 0) { - error_report("Failed to open file '%s'", tmpbuf); + error_report("Failed to open file '%s'", one_file); exit(1); } =20 mbs.mb_buf_size =3D TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_b= uf_size); mbs.mb_buf =3D g_realloc(mbs.mb_buf, mbs.mb_buf_size); =20 - load_image(tmpbuf, (unsigned char *)mbs.mb_buf + offs); + load_image(one_file, (unsigned char *)mbs.mb_buf + offs); mb_add_mod(&mbs, mbs.mb_buf_phys + offs, mbs.mb_buf_phys + offs + mb_mod_length, c); =20 @@ -347,6 +354,8 @@ int load_multiboot(FWCfgState *fw_cfg, (char *)mbs.mb_buf + offs, (char *)mbs.mb_buf + offs + mb_mod_length, c); initrd_filename =3D next_initrd+1; + g_free(one_file); + one_file =3D NULL; } while (not_last); } =20 diff --git a/include/qemu/option.h b/include/qemu/option.h index 1cfe5cb..3dfb449 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -28,7 +28,7 @@ =20 #include "qemu/queue.h" =20 -const char *get_opt_value(char *buf, int buf_size, const char *p); +const char *get_opt_value(const char *p, char **value); =20 void parse_option_size(const char *name, const char *value, uint64_t *ret, Error **errp); diff --git a/util/qemu-option.c b/util/qemu-option.c index fa1a9f1..58d1c23 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -70,25 +70,37 @@ static const char *get_opt_name(const char *p, char **o= ption, char delim) * delimiter is fixed to be comma which starts a new option. To specify an * option value that contains commas, double each comma. */ -const char *get_opt_value(char *buf, int buf_size, const char *p) +const char *get_opt_value(const char *p, char **value) { - char *q; + size_t capacity =3D 0, length; + const char *offset; + + *value =3D NULL; + while (1) { + offset =3D strchr(p, ','); + if (!offset) { + offset =3D p + strlen(p); + } =20 - q =3D buf; - while (*p !=3D '\0') { - if (*p =3D=3D ',') { - if (*(p + 1) !=3D ',') - break; - p++; + length =3D offset - p; + if (*offset !=3D '\0' && *(offset + 1) =3D=3D ',') { + length++; + } + if (value) { + *value =3D g_renew(char, *value, capacity + length + 1); + strncpy(*value + capacity, p, length); + (*value)[capacity + length] =3D '\0'; + } + capacity +=3D length; + if (*offset =3D=3D '\0' || + *(offset + 1) !=3D ',') { + break; } - if (q && (q - buf) < buf_size - 1) - *q++ =3D *p; - p++; + + p +=3D (offset - p) + 2; } - if (q) - *q =3D '\0'; =20 - return p; + return offset; } =20 static void parse_option_bool(const char *name, const char *value, bool *r= et, @@ -162,50 +174,43 @@ void parse_option_size(const char *name, const char *= value, =20 bool has_help_option(const char *param) { - size_t buflen =3D strlen(param) + 1; - char *buf =3D g_malloc(buflen); const char *p =3D param; bool result =3D false; =20 - while (*p) { - p =3D get_opt_value(buf, buflen, p); + while (*p && !result) { + char *value; + + p =3D get_opt_value(p, &value); if (*p) { p++; } =20 - if (is_help_option(buf)) { - result =3D true; - goto out; - } + result =3D is_help_option(value); + g_free(value); } =20 -out: - g_free(buf); return result; } =20 -bool is_valid_option_list(const char *param) +bool is_valid_option_list(const char *p) { - size_t buflen =3D strlen(param) + 1; - char *buf =3D g_malloc(buflen); - const char *p =3D param; - bool result =3D true; + char *value =3D NULL; + bool result =3D false; =20 while (*p) { - p =3D get_opt_value(buf, buflen, p); - if (*p && !*++p) { - result =3D false; + p =3D get_opt_value(p, &value); + if ((*p && !*++p) || + (!*value || *value =3D=3D ',')) { goto out; } =20 - if (!*buf || *buf =3D=3D ',') { - result =3D false; - goto out; - } + g_free(value); + value =3D NULL; } =20 + result =3D true; out: - g_free(buf); + g_free(value); return result; } =20 @@ -487,7 +492,7 @@ int qemu_opt_unset(QemuOpts *opts, const char *name) } } =20 -static void opt_set(QemuOpts *opts, const char *name, const char *value, +static void opt_set(QemuOpts *opts, const char *name, char *value, bool prepend, Error **errp) { QemuOpt *opt; @@ -496,6 +501,7 @@ static void opt_set(QemuOpts *opts, const char *name, c= onst char *value, =20 desc =3D find_desc_by_name(opts->list->desc, name); if (!desc && !opts_accepts_any(opts)) { + g_free(value); error_setg(errp, QERR_INVALID_PARAMETER, name); return; } @@ -509,8 +515,7 @@ static void opt_set(QemuOpts *opts, const char *name, c= onst char *value, QTAILQ_INSERT_TAIL(&opts->head, opt, next); } opt->desc =3D desc; - opt->str =3D g_strdup(value); - assert(opt->str); + opt->str =3D value; qemu_opt_parse(opt, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -521,7 +526,7 @@ static void opt_set(QemuOpts *opts, const char *name, c= onst char *value, void qemu_opt_set(QemuOpts *opts, const char *name, const char *value, Error **errp) { - opt_set(opts, name, value, false, errp); + opt_set(opts, name, g_strdup(value), false, errp); } =20 void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, @@ -755,7 +760,7 @@ static void opts_do_parse(QemuOpts *opts, const char *p= arams, const char *firstname, bool prepend, Error **err= p) { char *option =3D NULL; - char value[1024]; + char *value =3D NULL; const char *p,*pe,*pc; Error *local_err =3D NULL; =20 @@ -767,15 +772,15 @@ static void opts_do_parse(QemuOpts *opts, const char = *params, if (p =3D=3D params && firstname) { /* implicitly named first option */ option =3D g_strdup(firstname); - p =3D get_opt_value(value, sizeof(value), p); + p =3D get_opt_value(p, &value); } else { /* option without value, probably a flag */ p =3D get_opt_name(p, &option, ','); if (strncmp(option, "no", 2) =3D=3D 0) { memmove(option, option+2, strlen(option+2)+1); - pstrcpy(value, sizeof(value), "off"); + value =3D g_strdup("off"); } else { - pstrcpy(value, sizeof(value), "on"); + value =3D g_strdup("on"); } } } else { @@ -783,11 +788,12 @@ static void opts_do_parse(QemuOpts *opts, const char = *params, p =3D get_opt_name(p, &option, '=3D'); assert(*p =3D=3D '=3D'); p++; - p =3D get_opt_value(value, sizeof(value), p); + p =3D get_opt_value(p, &value); } if (strcmp(option, "id") !=3D 0) { /* store and parse */ opt_set(opts, option, value, prepend, &local_err); + value =3D NULL; if (local_err) { error_propagate(errp, local_err); goto cleanup; @@ -797,11 +803,13 @@ static void opts_do_parse(QemuOpts *opts, const char = *params, break; } g_free(option); - option =3D NULL; + g_free(value); + option =3D value =3D NULL; } =20 cleanup: g_free(option); + g_free(value); } =20 /** @@ -820,7 +828,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const c= har *params, bool permit_abbrev, bool defaults, Error **err= p) { const char *firstname; - char value[1024], *id =3D NULL; + char *id =3D NULL; const char *p; QemuOpts *opts; Error *local_err =3D NULL; @@ -829,11 +837,9 @@ static QemuOpts *opts_parse(QemuOptsList *list, const = char *params, firstname =3D permit_abbrev ? list->implied_opt_name : NULL; =20 if (strncmp(params, "id=3D", 3) =3D=3D 0) { - get_opt_value(value, sizeof(value), params+3); - id =3D value; + get_opt_value(params + 3, &id); } else if ((p =3D strstr(params, ",id=3D")) !=3D NULL) { - get_opt_value(value, sizeof(value), p+4); - id =3D value; + get_opt_value(p + 4, &id); } =20 /* @@ -845,6 +851,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const c= har *params, */ assert(!defaults || list->merge_lists); opts =3D qemu_opts_create(list, id, !defaults, &local_err); + g_free(id); if (opts =3D=3D NULL) { error_propagate(errp, local_err); return NULL; --=20 1.8.3.1