From patchwork Fri Jun 14 17:51:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 166870 Delivered-To: patch@linaro.org Received: by 2002:a92:4782:0:0:0:0:0 with SMTP id e2csp2351255ilk; Fri, 14 Jun 2019 10:53:32 -0700 (PDT) X-Google-Smtp-Source: APXvYqxzXDiRHjq/P/5qVnTSejOv9X9iLaYeqHW5lMtwefK+sWlAT7BgflwJzgBszD60qMPz6wNY X-Received: by 2002:a02:a90a:: with SMTP id n10mr53577752jam.61.1560534812194; Fri, 14 Jun 2019 10:53:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1560534812; cv=none; d=google.com; s=arc-20160816; b=yVqebnXXccH84bebFjcitQak8cVGHN3goY/9MPEsuEaEaEb0/WCbcfwO7U/TaLUI4s VTD/j289Q75TWoM7ctvxm4oH9ErXsG/PbgbaIKQAHJ7Y58o9ygSmUnsa/wQB/NvF3QPz eU20bSQi6DczMGxjGawm7TunjPIRU+K3CyGFket+gdUe+a7+394sZZhAl+sWOxzsrWSi I5b8tVEyE0QLTIjJaIfoAl5JcYZXRX1dSvHvjf1f6ckkM4S8eqq53VCrVGbWW01pWsxI KIagCilOiSDRkNhSZvmoHBvVX13tloirw3GIkFWvnBkHO/qM4A6wpAzeuS0ULnGHn9GW 7ypw== 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=jdpVZmVWQv0dglxKluP7KkrBSiyRCb1yf1g1wN+atyE=; b=Lw7bDwuWQb3KeuhD799xowE8q3YdUkZqXE4ID0lkm4NNs6gIxhTDTNCt8vcf6gmBRk Ds9nll/4QZNqEMGKMwgqeSYj5uIuhbTKqymMXJhCEp95a0d63JazqHUJwZVwdLaIr4RQ mdEICx9aOsmz4ZAPHJgZpyfCDwzi/aaI65GP2AxpO47ci9eMHULcpbliZI4/c2pEpWTb JdAc1bLfrlC6U/34yPLW68OnIEqARPuL4zrCMDPmmVQOPRA5VeL/3GFpPBGMew9ZpHhU qJIUpTggplBqi9hGx3oTyYKPg19kp17NWIXGIH8Id3FOI1oGghcFyTBuRYtr7KImrfg+ b/5A== 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 q8si4069982iob.45.2019.06.14.10.53.32 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 14 Jun 2019 10:53:32 -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 1hbqMr-0001pH-0V; Fri, 14 Jun 2019 17:52:01 +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 1hbqMo-0001nI-EA for xen-devel@lists.xenproject.org; Fri, 14 Jun 2019 17:51:58 +0000 X-Inumbo-ID: 1953a0f6-8ecd-11e9-939b-63e2fa18f954 Received: from foss.arm.com (unknown [217.140.110.172]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 1953a0f6-8ecd-11e9-939b-63e2fa18f954; Fri, 14 Jun 2019 17:51:55 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 75A492B; Fri, 14 Jun 2019 10:51:55 -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 8DB523F718; Fri, 14 Jun 2019 10:51:54 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Fri, 14 Jun 2019 18:51:38 +0100 Message-Id: <20190614175144.20046-4-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190614175144.20046-1-julien.grall@arm.com> References: <20190614175144.20046-1-julien.grall@arm.com> Subject: [Xen-devel] [PATCH MM-PART3 v3 3/9] 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 Reviewed-by: Stefano Stabellini --- Changes in v3: - Only allow modification on valid entry 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 | 122 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 104 insertions(+), 18 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index b13d9adf40..dcf041578b 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -50,6 +50,19 @@ #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)] @@ -941,12 +954,81 @@ 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 modifying an invalid entry. */ + if ( !lpae_is_valid(entry) ) + { + mm_printk("Modifying invalid entry is not allowed.\n"); + return false; + } + + /* 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) ) { @@ -962,15 +1044,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)); @@ -982,12 +1061,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 @@ -995,12 +1068,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; @@ -1022,6 +1089,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 )