From nobody Sat Apr 27 03:52:33 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1557837151; cv=none; d=zoho.com; s=zohoarc; b=MF3y6cGhfaykEHcvQ7I2IFgzhe/9TU/vloyLU8q2dIKMLGHlmOgcyWfF0AIH9Z98wEPlAKhwXiZRuEOKiBiylhuyMinwoofwJ7zPQzNemm4gfbaEqxQN1mMe1zUVg/0+9/0ci0MziOzxqexpO8dW5WQJ+PRHYLl8a2uz1W2TZJw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1557837151; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=rYohg+P0gAZ9Q0QlMGjYyMkis+pGx79Rxxhq+p8NUg0=; b=jIBQ1O7eTbF5J69fR/gVzHczYsOqt+LNl0WP0/moizUyCtbWnav/j0TA25vIQQTYJaSZBKWDrDa4WpUmYEkoJcuMrBB4zV1nV4R36tG5+9ij3/LcPSdKALWt1BNnupCRdvZOGUD5xS5ekEWmWUqBrpHBM6V1Da1Q/+8j8KeiD1Y= ARC-Authentication-Results: i=1; mx.zoho.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1557837150923421.80877335315984; Tue, 14 May 2019 05:32:30 -0700 (PDT) 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 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) X-Inumbo-ID: 39653716-7644-11e9-99f3-63d46a5efb4e 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 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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)) =20 +#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)] =20 @@ -968,12 +981,74 @@ enum xenmap_operation { RESERVE }; =20 +/* 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 !=3D 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 allo= wed.\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 (%#"P= RI_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)) =3D=3D 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 allow= ed.\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 add= r, mfn_t mfn, unsigned int flags) { lpae_t pte, *entry; lpae_t *third =3D NULL; =20 + /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */ + ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) !=3D (_PAGE_POPULATE|_= PAGE_PRESENT)); + entry =3D &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 =3D mfn_to_virt(lpae_get_mfn(*entry)); entry =3D &third[third_table_offset(addr)]; =20 + 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=3D%= lx mfn=3D%"PRI_mfn"\n", - __func__, addr, mfn_x(mfn)); - return -EINVAL; - } if ( op =3D=3D RESERVE ) break; pte =3D 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=3D%lx= \n", - __func__, op =3D=3D REMOVE ? "remove" : "modify", a= ddr); - return -EINVAL; - } if ( op =3D=3D REMOVE ) pte.bits =3D 0; else @@ -1022,12 +1088,6 @@ static int xen_pt_update_entry(enum xenmap_operation= op, unsigned long addr, pte =3D *entry; pte.pt.ro =3D PAGE_RO_MASK(flags); pte.pt.xn =3D PAGE_XN_MASK(flags); - if ( !pte.pt.ro && !pte.pt.xn ) - { - printk("%s: Incorrect combination for addr=3D%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 =3D 0; unsigned long addr =3D virt, addr_end =3D addr + nr_mfns * PAGE_SIZE; =20 + /* + * 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); =20 for( ; addr < addr_end; addr +=3D PAGE_SIZE ) --=20 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel