From patchwork Tue May 14 12:31:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 164200 Delivered-To: patch@linaro.org Received: by 2002:a92:9e1a:0:0:0:0:0 with SMTP id q26csp2561872ili; Tue, 14 May 2019 05:32:40 -0700 (PDT) X-Google-Smtp-Source: APXvYqwDY1apmvksuB95T8nHSEoARb4qvIixoJvYK4sJ/pz2fIblIk8pp2eAeOVoYFt5q3L09rDP X-Received: by 2002:a5d:85d9:: with SMTP id e25mr18994352ios.26.1557837160690; Tue, 14 May 2019 05:32:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1557837160; cv=none; d=google.com; s=arc-20160816; b=Ea+8shFCIM/YY0qhD/dlLPUU8rb8fOtG+AJn1hZNRCYmuMNR0tnEELwX9XSD2yj/Kz TsEkAUOSb/N2fJgJEm47PeHDXkIq2LMLeZ6Ynz/H4UBUd9aqUkN1AZpGrCLSu7WCYegu 2nSwdZafxTq4h454lEuJC8IKMHjmuWLyfiIxBppcZ8tfN8CNSOB5AN2tuNVSEQaa8BsB wFCN3AtIqyEXy5uHZdtfiBhAS3VmaBXOr/fhbsjGj3j4tta3OD19sID5lMXLwStt5PTM fwsKtGnMa6GYMFkyXp/3DE0Jc0TXNEYkTM54sPp5kgsaspBBPObsjlWY/MJWmTmyC1wp 9PVw== 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=rYohg+P0gAZ9Q0QlMGjYyMkis+pGx79Rxxhq+p8NUg0=; b=RVlBjA8vGTjn3spy9tNIu0ootkblH8X0njGsquqL0990VA28kHMnhvqxgNCe55sihk J8XtYdt32SPJpdBm3eweZpWqAOt64tbNK4UgenPr1iD3yCnGc+9adOc2YYL6oAgcvDAC YFf7pOxFbauyHjKa2NXi83f92NuFoXJw9SzcOeLkyuDnosJ6wWT5gks6DWMCDMwazeCG YiMdnSyXdOezHY1XuxMuDBV+U+2NASUcolcAG/EUo/1jaA1GyPY2Iqf79NaCkRbGOHbP oTH5vFVVYoXTWeqTh1nTjvXGx019xoynJB+e7fNVvZr47iB/ZbOU1tLJXRqmDbXnx1Wa 6KGw== 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 i203si1456914iti.6.2019.05.14.05.32.40 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 14 May 2019 05:32:40 -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 1hQWar-0005N5-V0; Tue, 14 May 2019 12:31:41 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hQWar-0005Mx-8b for xen-devel@lists.xenproject.org; Tue, 14 May 2019 12:31:41 +0000 X-Inumbo-ID: 39653716-7644-11e9-99f3-63d46a5efb4e Received: from foss.arm.com (unknown [217.140.101.70]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 39653716-7644-11e9-99f3-63d46a5efb4e; Tue, 14 May 2019 12:31:40 +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 0ED03341; Tue, 14 May 2019 05:31:40 -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 CF54E3F71E; Tue, 14 May 2019 05:31:38 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Tue, 14 May 2019 13:31:19 +0100 Message-Id: <20190514123125.29086-7-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190514123125.29086-1-julien.grall@arm.com> References: <20190514123125.29086-1-julien.grall@arm.com> Subject: [Xen-devel] [PATCH MM-PART3 v2 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 , Stefano Stabellini , Andrii Anisov 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 --- Changes in v2: - Correctly detect the removal of a page - Fix ASSERT on flags in the else case - Add Andrii's reviewed-by --- 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 2192dede55..45a6f9287f 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 + #define DEFINE_PAGE_TABLES(name, nr) \ lpae_t __aligned(PAGE_SIZE) name[LPAE_ENTRIES * (nr)] @@ -968,12 +981,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) ) { @@ -989,15 +1064,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)); @@ -1009,12 +1081,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 @@ -1022,12 +1088,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; @@ -1049,6 +1109,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 )