From patchwork Tue Dec 15 15:04:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juergen Gross X-Patchwork-Id: 11974945 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E85BC4361B for ; Tue, 15 Dec 2020 15:04:32 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 004A022525 for ; Tue, 15 Dec 2020 15:04:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 004A022525 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.54314.94285 (Exim 4.92) (envelope-from ) id 1kpBsA-0001MS-Vr; Tue, 15 Dec 2020 15:04:18 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 54314.94285; Tue, 15 Dec 2020 15:04:18 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kpBsA-0001ML-Sv; Tue, 15 Dec 2020 15:04:18 +0000 Received: by outflank-mailman (input) for mailman id 54314; Tue, 15 Dec 2020 15:04:16 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kpBs8-0001MG-MR for xen-devel@lists.xenproject.org; Tue, 15 Dec 2020 15:04:16 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 2348941b-46a8-412d-b803-75683c9fd45f; Tue, 15 Dec 2020 15:04:15 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id D4C48AC7F; Tue, 15 Dec 2020 15:04:14 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 2348941b-46a8-412d-b803-75683c9fd45f X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1608044655; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=1ONxP5uPtKWFefZOyd/Uymh+C2zg0SXGzfp1xar6tXg=; b=eT/aDkZ+5/0+ZflH9JfPI+zJmA6sc3gawUouS0XEtySeQ77MT4yCRKtUYqHUsNIkilx8+q AGwEOoctpwnibcMFEdrJFs+38I+ab3eY/TuQQAdg7hnjNgiGLYg4yMzIqwvL9BaApF8x6E caZpu2LJ7eiTqzUs9M8+0I2LerOJiTY= From: Juergen Gross To: xen-devel@lists.xenproject.org Cc: Juergen Gross , Ian Jackson , Wei Liu , Paul Durrant , Julien Grall Subject: [PATCH] tools/xenstore: rework path length check Date: Tue, 15 Dec 2020 16:04:11 +0100 Message-Id: <20201215150411.9987-1-jgross@suse.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 The different fixed limits for absolute and relative path lengths of Xenstore nodes make it possible to create per-domain nodes via absolute paths which are not accessible using relative paths, as the two limits differ by 1024 characters. Instead of this weird limits use only one limit, which applies to the relative path length of per-domain nodes and to the absolute path length of all other nodes. This means, the path length check is applied to the path after removing a possible start of "/local/domain//" with being a domain id. There has been the request to be able to limit the path lengths even more, so an additional quota is added which can be applied to path lengths. It is XENSTORE_REL_PATH_MAX (2048) per default, but can be set to lower values. This is done via the new "-M" or "--path-max" option when invoking xenstored. Signed-off-by: Juergen Gross Reviewed-by: Paul Durrant Acked-by: Julien Grall Reviewed-by: Andrew Cooper Acked-by: Wei Liu --- This patch was originally thought to be part of XSA-323, but later it was decided not to include it, as in C Xenstored this is no security issue. --- tools/xenstore/xenstored_core.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 746a1247b3..3082a36d3a 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -102,6 +102,7 @@ int quota_nb_watch_per_domain = 128; int quota_max_entry_size = 2048; /* 2K */ int quota_max_transaction = 10; int quota_nb_perms_per_node = 5; +int quota_max_path_len = XENSTORE_REL_PATH_MAX; void trace(const char *fmt, ...) { @@ -734,6 +735,9 @@ static bool valid_chars(const char *node) bool is_valid_nodename(const char *node) { + int local_off = 0; + unsigned int domid; + /* Must start in /. */ if (!strstarts(node, "/")) return false; @@ -746,7 +750,10 @@ bool is_valid_nodename(const char *node) if (strstr(node, "//")) return false; - if (strlen(node) > XENSTORE_ABS_PATH_MAX) + if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1) + local_off = 0; + + if (strlen(node) > local_off + quota_max_path_len) return false; return valid_chars(node); @@ -806,6 +813,8 @@ static struct node *get_node_canonicalized(struct connection *conn, if (!canonical_name) canonical_name = &tmp_name; *canonical_name = canonicalize(conn, ctx, name); + if (!*canonical_name) + return NULL; return get_node(conn, ctx, *canonical_name, perm); } @@ -1926,6 +1935,7 @@ static void usage(void) " -W, --watch-nb limit the number of watches per domain,\n" " -t, --transaction limit the number of transaction allowed per domain,\n" " -A, --perm-nb limit the number of permissions per node,\n" +" -M, --path-max limit the allowed Xenstore node path length,\n" " -R, --no-recovery to request that no recovery should be attempted when\n" " the store is corrupted (debug only),\n" " -I, --internal-db store database in memory, not on disk\n" @@ -1947,6 +1957,7 @@ static struct option options[] = { { "trace-file", 1, NULL, 'T' }, { "transaction", 1, NULL, 't' }, { "perm-nb", 1, NULL, 'A' }, + { "path-max", 1, NULL, 'M' }, { "no-recovery", 0, NULL, 'R' }, { "internal-db", 0, NULL, 'I' }, { "verbose", 0, NULL, 'V' }, @@ -1969,7 +1980,7 @@ int main(int argc, char *argv[]) int timeout; - while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:A:T:RVW:", options, + while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:A:M:T:RVW:", options, NULL)) != -1) { switch (opt) { case 'D': @@ -2014,6 +2025,10 @@ int main(int argc, char *argv[]) case 'A': quota_nb_perms_per_node = strtol(optarg, NULL, 10); break; + quota_max_path_len = strtol(optarg, NULL, 10); + quota_max_path_len = min(XENSTORE_REL_PATH_MAX, + quota_max_path_len); + break; case 'e': dom0_event = strtol(optarg, NULL, 10); break;