From patchwork Tue May 8 22:14:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 910532 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="okwfziGd"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 40gZC94sZrz9s3D for ; Wed, 9 May 2018 08:38:28 +1000 (AEST) Received: from localhost ([::1]:53637 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGBFY-0006qT-Ic for incoming@patchwork.ozlabs.org; Tue, 08 May 2018 18:38:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41317) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGAt3-000331-U2 for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fGAt2-0001Pf-I2 for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:09 -0400 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]:37213) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fGAt2-0001Op-7f for qemu-devel@nongnu.org; Tue, 08 May 2018 18:15:08 -0400 Received: by mail-wm0-x243.google.com with SMTP id l1-v6so24209581wmb.2 for ; Tue, 08 May 2018 15:15:08 -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=ZcUbzRkmqIqlOegwKYPCk6KEdKVzvrj32YeNN6ROJzo=; b=okwfziGdlUVLeGRDUa8Aqiaoy3sBpQZwYh41gmcG0q6I1UzqR8tcnwrmB0h8k9+0F8 tv1lE8KBMcyww3SWQAqAZ1IwsLUThTapQC0M1IszqyTPwDBbmUnqDOUvRy+0ClsCcH/M /j3qpGIzNmxTXvpfAABPciOR6ljE8TeGZVXsQdEg2yh8aHpsGoXCBJO6ibvcZcA2LLFI eQct0Mh5CP1gTOfjVPtSeet9OuYDGWm8QnXNU0GInyn4P3VcHxAXZejybKoyTKXsZQND RulSqe7SlMlSMhmCGr6OoH2QSO+s1L7GIxAUDfsLXkEvBcr3FjRrg62nKY+rgJhaT2TS UYGg== 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=ZcUbzRkmqIqlOegwKYPCk6KEdKVzvrj32YeNN6ROJzo=; b=q6o1lLnAkTV2TTlR2UITDzVyqaD14hIlzDjkCSHWRyosW2hci+HgHQJDFS51QfMXP1 3Phrlt3u1gzR9nndrCmx8/OUcIVrG0QBIio07Rw75g5Um/SLYjCiVWto7zL4ukRUuaNc 6pQPG10emGx8L36Jo4mFvfCpyzWbDfk2xSZR6qNXtbuDPHSjNREtuaY3OuqqvQXs3V25 SAVVwG9hOF9iSDyAjgFQoFdpOvsldnu7msZO3Q2ExUc75sM7jSdPy0kttx06Wa9AgHmW pNGgh5P2zqK/sym/89TSqgrhFvh5Pi2373HUQSKCzP6iZLl8jRe79DW3pp4c5FhpunQc jHLg== X-Gm-Message-State: ALQs6tA2TuD4hHw45MXzzHKqc0xIYgSd3zfTihQe4M4mQtBgsKZVgi2X 2UHQ1+lfdCeyP/vU42l/rAM3OzOS X-Google-Smtp-Source: AB8JxZp6ishB0/KYjZuch6Fu1MczOlvSZQAmpXl/mcLJmapQnBK/bnws7YCn6NHXcCwFHSOGAiYUuA== X-Received: by 2002:a50:8307:: with SMTP id 7-v6mr56881812edh.263.1525817706935; Tue, 08 May 2018 15:15:06 -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.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 May 2018 15:15:06 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 9 May 2018 00:14:31 +0200 Message-Id: <1525817687-34620-15-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 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:400c:c09::243 Subject: [Qemu-devel] [PULL 14/30] opts: don't silently truncate long parameter keys 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Daniel P. Berrangé The existing QemuOpts parsing code uses a fixed size 128 byte buffer for storing the parameter keys. If a key exceeded this size it was silently truncate and no error reported to the user. This behaviour was reasonable & harmless because traditionally the key names are all statically declared, and it was known that no code was declaring a key longer than 127 bytes. This assumption, however, ceased to be valid once the block layer added support for dot-separate compound keys. This syntax allows for keys that can be arbitrarily long, limited only by the number of block drivers you can stack up. With this usage, silently truncating the key name can never lead to correct behaviour. Hopefully such truncation would turn into an error, when the block code then tried to extract options later, but there's no guarantee that will happen. It is conceivable that an option specified by the user may be truncated and then ignored. This could have serious consequences, possibly even leading to security problems if the ignored option set a security relevant parameter. 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 keys during parsing, lifting the arbitrary length restriction. Signed-off-by: Daniel P. Berrangé Message-Id: <20180416111743.8473-3-berrange@redhat.com> Signed-off-by: Paolo Bonzini Signed-off-by: Daniel P. Berrangé --- tests/test-qemu-opts.c | 18 ------------------ util/qemu-option.c | 44 ++++++++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 40 deletions(-) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 77dd72b..7092e21 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -459,8 +459,6 @@ static void test_opts_parse(void) { Error *err = NULL; QemuOpts *opts; - char long_key[129]; - char *params; /* Nothing */ opts = qemu_opts_parse(&opts_list_03, "", false, &error_abort); @@ -471,22 +469,6 @@ static void test_opts_parse(void) g_assert_cmpuint(opts_count(opts), ==, 1); g_assert_cmpstr(qemu_opt_get(opts, ""), ==, "val"); - /* Long key */ - memset(long_key, 'a', 127); - long_key[127] = 'z'; - long_key[128] = 0; - params = g_strdup_printf("%s=v", long_key); - opts = qemu_opts_parse(&opts_list_03, params + 1, NULL, &error_abort); - g_assert_cmpuint(opts_count(opts), ==, 1); - g_assert_cmpstr(qemu_opt_get(opts, long_key + 1), ==, "v"); - - /* Overlong key gets truncated */ - opts = qemu_opts_parse(&opts_list_03, params, NULL, &error_abort); - g_assert(opts_count(opts) == 1); - long_key[127] = 0; - g_assert_cmpstr(qemu_opt_get(opts, long_key), ==, "v"); - g_free(params); - /* Multiple keys, last one wins */ opts = qemu_opts_parse(&opts_list_03, "a=1,b=2,,x,a=3", false, &error_abort); diff --git a/util/qemu-option.c b/util/qemu-option.c index baca40f..fa1a9f1 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -43,27 +43,23 @@ * first byte of the option name) * * The option name is delimited by delim (usually , or =) or the string end - * and is copied into buf. If the option name is longer than buf_size, it is - * truncated. buf is always zero terminated. + * and is copied into option. The caller is responsible for free'ing option + * when no longer required. * * The return value is the position of the delimiter/zero byte after the option * name in p. */ -static const char *get_opt_name(char *buf, int buf_size, const char *p, - char delim) +static const char *get_opt_name(const char *p, char **option, char delim) { - char *q; + char *offset = strchr(p, delim); - q = buf; - while (*p != '\0' && *p != delim) { - if (q && (q - buf) < buf_size - 1) - *q++ = *p; - p++; + if (offset) { + *option = g_strndup(p, offset - p); + return offset; + } else { + *option = g_strdup(p); + return p + strlen(p); } - if (q) - *q = '\0'; - - return p; } /* @@ -758,7 +754,8 @@ void qemu_opts_print(QemuOpts *opts, const char *separator) static void opts_do_parse(QemuOpts *opts, const char *params, const char *firstname, bool prepend, Error **errp) { - char option[128], value[1024]; + char *option = NULL; + char value[1024]; const char *p,*pe,*pc; Error *local_err = NULL; @@ -769,11 +766,11 @@ static void opts_do_parse(QemuOpts *opts, const char *params, /* found "foo,more" */ if (p == params && firstname) { /* implicitly named first option */ - pstrcpy(option, sizeof(option), firstname); + option = g_strdup(firstname); p = get_opt_value(value, sizeof(value), p); } else { /* option without value, probably a flag */ - p = get_opt_name(option, sizeof(option), p, ','); + p = get_opt_name(p, &option, ','); if (strncmp(option, "no", 2) == 0) { memmove(option, option+2, strlen(option+2)+1); pstrcpy(value, sizeof(value), "off"); @@ -783,10 +780,8 @@ static void opts_do_parse(QemuOpts *opts, const char *params, } } else { /* found "foo=bar,more" */ - p = get_opt_name(option, sizeof(option), p, '='); - if (*p != '=') { - break; - } + p = get_opt_name(p, &option, '='); + assert(*p == '='); p++; p = get_opt_value(value, sizeof(value), p); } @@ -795,13 +790,18 @@ static void opts_do_parse(QemuOpts *opts, const char *params, opt_set(opts, option, value, prepend, &local_err); if (local_err) { error_propagate(errp, local_err); - return; + goto cleanup; } } if (*p != ',') { break; } + g_free(option); + option = NULL; } + + cleanup: + g_free(option); } /**