From patchwork Wed Apr 24 16:59:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 162796 Delivered-To: patch@linaro.org Received: by 2002:a02:c6d8:0:0:0:0:0 with SMTP id r24csp895752jan; Wed, 24 Apr 2019 10:01:57 -0700 (PDT) X-Google-Smtp-Source: APXvYqwENAGpmwhQXZ+gzBXg+Quw4rCp7Unp7R4/TvGIMy63esoUZRGr2W05ghXLth7CDD4r1fTl X-Received: by 2002:a25:1254:: with SMTP id 81mr27113087ybs.237.1556125317408; Wed, 24 Apr 2019 10:01:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1556125317; cv=none; d=google.com; s=arc-20160816; b=sL/hx9mM5BvutRJM90oSbXUJAdzFd5FgbP4e/vl1/5MavIRcejnWowg5XUplLNVDs9 3JivK6Hq2DGBmwknixgLmq1ziDLPptfR07PIu+9l3qXg1Pz2osqkihUoWC6rRxAt9e/m FLd/CpEduZ6P20iRdDK/p9Wil9TCIeeDvWNIEe1R9NHZEAX/SFc6qM1BW6C1P8+jwZq7 On9rSOtCzW/ug9eCxpY8EpsFptVOh1FTEL2xH+4Q1swmvPVPpnY5t+A88jJM0HoSU9// XY8H+0ZuLqb8HqECjOmsZ/DsDckHTXM3EF74anK/7G3cjPim9URQ3/7SL1OCADDsz6dx xyIQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-unsubscribe:list-id :precedence:subject:references:in-reply-to:message-id:date:to:from; bh=lpedV1eiG5rJLQ2LjvVRzgM1eOhn2vZtAOxQCzzBEsQ=; b=t/jj6GJmykJXYUDUiKB7pIME8iwNzwiUTy6uyvdZBiBpt6v2vQr1kt1hLcZDEXPXJj 2aPhbDYZ8ZId5UiTcfc99RNLxFfqLBvhDDLpYWx2XzBwVKJQVtO489TBD2M2At6r6J4v xIPeXlvAUnw3J/b8vpWcMxLuJ2G/52buzQ1BygwlC1waxiSJhN7qqf5t12uZGDxPp/8Q 1arumolmmZ2hWFOWNvAVFLqDcdtolz1FHjzsP0qCjl504PUTYuuTqI+I43y/oe2/V6gz AIk++z1G4mxb84HoBc+V6+sIAWejtH4ykG7956qrrRxaPqZutHSIYlmxapiF4zZPvl3w d+IA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id a205si3441805ywc.284.2019.04.24.10.01.57 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Apr 2019 10:01:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hJLFn-0007o1-Jr; Wed, 24 Apr 2019 17:00:15 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hJLFm-0007ne-Fl for xen-devel@lists.xenproject.org; Wed, 24 Apr 2019 17:00:14 +0000 X-Inumbo-ID: 6cdf9cf3-66b2-11e9-92d7-bc764e045a96 Received: from foss.arm.com (unknown [217.140.101.70]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTP id 6cdf9cf3-66b2-11e9-92d7-bc764e045a96; Wed, 24 Apr 2019 17:00:12 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B600F374; Wed, 24 Apr 2019 10:00:12 -0700 (PDT) Received: from e108454-lin.cambridge.arm.com (e108454-lin.cambridge.arm.com [10.1.196.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9B6743F557; Wed, 24 Apr 2019 10:00:11 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Wed, 24 Apr 2019 17:59:49 +0100 Message-Id: <20190424165955.23718-7-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190424165955.23718-1-julien.grall@arm.com> References: <20190424165955.23718-1-julien.grall@arm.com> Subject: [Xen-devel] [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Oleksandr_Tyshchenko@epam.com, Julien Grall , sstabellini@kernel.org, Andrii_Anisov@epam.com MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" The code handling Xen PT update has quite a few restrictions on what it can do. This is not a bad thing as it keeps the code simple. There are already a few checks scattered in current page table handling. However they are not sufficient as they could still allow to modify/remove entry with contiguous bit set. The checks are divided in two sets: - per entry check: They are gathered in a new function that will check whether an update is valid based on the flags passed and the current value of an entry. - global check: They are sanity check on xen_pt_update() parameters. Additionally to contiguous check, we also now check that the caller is not trying to modify the memory attributes of an entry. Lastly, it was probably a bit over the top to forbid removing an invalid mapping. This could just be ignored. The new behavior will be helpful in future changes. Signed-off-by: Julien Grall Reviewed-by: Andrii Anisov --- xen/arch/arm/mm.c | 115 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 97 insertions(+), 18 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index eee7122c88..5eb6f47d74 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -50,6 +50,19 @@ struct domain *dom_xen, *dom_io, *dom_cow; #undef mfn_to_virt #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn)) +#ifdef NDEBUG +static inline void +__attribute__ ((__format__ (__printf__, 1, 2))) +mm_printk(const char *fmt, ...) {} +#else +#define mm_printk(fmt, args...) \ + do \ + { \ + dprintk(XENLOG_ERR, fmt, ## args); \ + WARN(); \ + } while (0); +#endif + /* Static start-of-day pagetables that we use before the allocators * are up. These are used by all CPUs during bringup before switching * to the CPUs own pagetables. @@ -963,12 +976,74 @@ enum xenmap_operation { RESERVE }; +/* Sanity check of the entry */ +static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags) +{ + /* Sanity check when modifying a page. */ + if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) ) + { + /* We don't allow changing memory attributes. */ + if ( entry.pt.ai != PAGE_AI_MASK(flags) ) + { + mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n", + entry.pt.ai, PAGE_AI_MASK(flags)); + return false; + } + + /* We don't allow modifying entry with contiguous bit set. */ + if ( entry.pt.contig ) + { + mm_printk("Modifying entry with contiguous bit set is not allowed.\n"); + return false; + } + } + /* Sanity check when inserting a page */ + else if ( flags & _PAGE_PRESENT ) + { + /* We should be here with a valid MFN. */ + ASSERT(!mfn_eq(mfn, INVALID_MFN)); + + /* We don't allow replacing any valid entry. */ + if ( lpae_is_valid(entry) ) + { + mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n", + mfn_x(lpae_get_mfn(entry)), mfn_x(mfn)); + return false; + } + } + /* Sanity check when removing a page. */ + else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 ) + { + /* We should be here with an invalid MFN. */ + ASSERT(mfn_eq(mfn, INVALID_MFN)); + + /* We don't allow removing page with contiguous bit set. */ + if ( entry.pt.contig ) + { + mm_printk("Removing entry with contiguous bit set is not allowed.\n"); + return false; + } + } + /* Sanity check when populating the page-table. No check so far. */ + else + { + ASSERT(!(flags & _PAGE_POPULATE)); + /* We should be here with an invalid MFN */ + ASSERT(mfn_eq(mfn, INVALID_MFN)); + } + + return true; +} + static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr, mfn_t mfn, unsigned int flags) { lpae_t pte, *entry; lpae_t *third = NULL; + /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */ + ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT)); + entry = &xen_second[second_linear_offset(addr)]; if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) ) { @@ -984,15 +1059,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr, third = mfn_to_virt(lpae_get_mfn(*entry)); entry = &third[third_table_offset(addr)]; + if ( !xen_pt_check_entry(*entry, mfn, flags) ) + return -EINVAL; + switch ( op ) { case INSERT: case RESERVE: - if ( lpae_is_valid(*entry) ) - { - printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n", - __func__, addr, mfn_x(mfn)); - return -EINVAL; - } if ( op == RESERVE ) break; pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags)); @@ -1004,12 +1076,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr, break; case MODIFY: case REMOVE: - if ( !lpae_is_valid(*entry) ) - { - printk("%s: trying to %s a non-existing mapping addr=%lx\n", - __func__, op == REMOVE ? "remove" : "modify", addr); - return -EINVAL; - } if ( op == REMOVE ) pte.bits = 0; else @@ -1017,12 +1083,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr, pte = *entry; pte.pt.ro = PAGE_RO_MASK(flags); pte.pt.xn = PAGE_XN_MASK(flags); - if ( !pte.pt.ro && !pte.pt.xn ) - { - printk("%s: Incorrect combination for addr=%lx\n", - __func__, addr); - return -EINVAL; - } } write_pte(entry, pte); break; @@ -1044,6 +1104,25 @@ static int xen_pt_update(enum xenmap_operation op, int rc = 0; unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE; + /* + * The hardware was configured to forbid mapping both writeable and + * executable. + * When modifying/creating mapping (i.e _PAGE_PRESENT is set), + * prevent any update if this happen. + */ + if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) && + !PAGE_XN_MASK(flags) ) + { + mm_printk("Mappings should not be both Writeable and Executable.\n"); + return -EINVAL; + } + + if ( !IS_ALIGNED(virt, PAGE_SIZE) ) + { + mm_printk("The virtual address is not aligned to the page-size.\n"); + return -EINVAL; + } + spin_lock(&xen_pt_lock); for( ; addr < addr_end; addr += PAGE_SIZE )