From patchwork Mon Aug 12 20:27:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 171117 Delivered-To: patch@linaro.org Received: by 2002:a92:d204:0:0:0:0:0 with SMTP id y4csp10965ily; Mon, 12 Aug 2019 13:28:52 -0700 (PDT) X-Google-Smtp-Source: APXvYqwB6VsPNAGUkJkyz07BRn26sdxB8KSdN1MXxmOBpmzdOvekt/X1lzAtqQ/br5bHvR/Jcrua X-Received: by 2002:a5e:db47:: with SMTP id r7mr752084iop.184.1565641732054; Mon, 12 Aug 2019 13:28:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1565641732; cv=none; d=google.com; s=arc-20160816; b=koD7yAifnu+TLOhlJtAsHOHmHz3UAlQhAtzK/A5QHlY2LMa67mpGIv+CYhWcH6c2dC C2d1e9whdkID2az3wxfq3z3TgP/i/8L5onYyFLKHuEzF9LYLSpKEUuthdxeClUaVGE2/ HkkJivntJRNiF91ZoLYXFklF67lADSCIAdmvWqrJrDxgWszrz1sEWTquMLYvrx+p3CR/ V68w43yI+BoC+4qZ7bOgCeYCCMBJP1xBI/iO63xJE3sVnmHe0JNhABdLoR8h0TlMEtLt qScIyJGCs2baCbKn4x+WHnZEvWSrHZov7HV+NNgI9c8T8EDO9L4lzMteWE1sHO7aq2FG NcKg== 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:message-id:date:to:from; bh=bg3GHyWvR6Qk82mFrlFhFJivzCFSdofv3UxJ1i7jl+A=; b=rnlzGClKKSCUf/PcbGPqVUbNd9nLd8pqd9oggk7BIaotCCta+W/T+BaNOydoCHDnAU G/RjYKsq4g7RtUF9yTl5AmPD+WR78jShJmtxicDIhZhDx5oK1gsIrYs//drtbz1+V1St 1mcFhRfvg0AydoQdTgT43qcpQPeZq2l91x2e0JbbQg1UdBZ4iWKua6TiCEkN3wMhiWd6 VM9dWEf/+jC+HMfaPSIHA3orRUzuikqxNzw2C4iDmn6Jq56JhX32f+HLFYCg7yjeZUVJ Or4K+31inbpQ1KTpJWY+DMT6nrgkuoBEcr0pQ6t9mH8N0vtUF7eD/5xsUnoPV04WEM7K ttdQ== 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 l14si24128559jap.53.2019.08.12.13.28.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 12 Aug 2019 13:28:52 -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 1hxGv0-0006gz-6L; Mon, 12 Aug 2019 20:27:50 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hxGuz-0006gu-8z for xen-devel@lists.xenproject.org; Mon, 12 Aug 2019 20:27:49 +0000 X-Inumbo-ID: a5f1e40c-bd3f-11e9-8980-bc764e045a96 Received: from foss.arm.com (unknown [217.140.110.172]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTP id a5f1e40c-bd3f-11e9-8980-bc764e045a96; Mon, 12 Aug 2019 20:27:47 +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 73EDF15A2; Mon, 12 Aug 2019 13:27:47 -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 8BFFA3F718; Mon, 12 Aug 2019 13:27:46 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Mon, 12 Aug 2019 21:27:35 +0100 Message-Id: <20190812202735.23411-1-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 Subject: [Xen-devel] [PATCH] xen/arm: p2m: Free the p2m entry after flushing the IOMMU TLBs 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: olekstysh@gmail.com, oleksandr_tyshchenko@epam.com, Julien Grall , Stefano Stabellini , Volodymyr Babchuk MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" When freeing a p2m entry, all the sub-tree behind it will also be freed. This may include intermediate page-tables or any l3 entry requiring to drop a reference (e.g for foreign pages). As soon as pages are freed, they may be re-used by Xen or another domain. Therefore it is necessary to flush *all* the TLBs beforehand. While CPU TLBs will be flushed before freeing the pages, this is not the case for IOMMU TLBs. This can be solved by moving the IOMMU TLBs flush earlier in the code. This wasn't considered as a security issue as device passthrough on Arm is not security supported. Signed-off-by: Julien Grall Tested-by: Oleksandr Tyshchenko Reviewed-by: Stefano Stabellini --- Cc: olekstysh@gmail.com Cc: oleksandr_tyshchenko@epam.com I discovered it while looking at the code, so I don't have any reproducer of the issue. There is a small windows where page could be reallocated to Xen or another domain but still present in the IOMMU TLBs. This patch only address the case where the flush succeed. In the unlikely case where it does not succeed, then we will still free the pages. The IOMMU helper will crash domain, but the device may still not be quiescent. So there are a potentially issues do DMA on wrong things. At the moment, none of the Arm IOMMUs drivers (including the IPMMU one under review) are return an error here. Note that flush may still fail (see timeout), but is ignored. This is not great as it means a device may DMA into something that does not belong to the domain. So we probably want to return an error here. Even if an error is returned, there are still potential issues (see above). The fix is not entirely trivial, we would need to keep the page around until the a device is quiescent or the IOMMU is reset. This mostly likely means until the domain is fully destroyed. One of the solution would be to: 1) Have a pool of memory for each domain p2m page-tables. So the domain can only touch itself 2) Defer foreign mapping removal 1) should also solve the case where the P2M is trying to shatter everything and therefore hog the memory. Note that today we don't free empty page-tables. 2) I always felt trying to remove the foreign page reference in the p2m code was wrong. This is done because we currently allow the guest to remove any mapping. So we need to protect ourself against a rogue guest. We could try to restrict what the guest can do on the p2m. --- xen/arch/arm/p2m.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 3c8287a048..963cd1d600 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1048,14 +1048,6 @@ static int __p2m_set_entry(struct p2m_domain *p2m, p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, sgfn); } - /* - * Free the entry only if the original pte was valid and the base - * is different (to avoid freeing when permission is changed). - */ - if ( p2m_is_valid(orig_pte) && - !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) ) - p2m_free_entry(p2m, orig_pte, level); - if ( has_iommu_pt(p2m->domain) && (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) ) { @@ -1072,6 +1064,14 @@ static int __p2m_set_entry(struct p2m_domain *p2m, else rc = 0; + /* + * Free the entry only if the original pte was valid and the base + * is different (to avoid freeing when permission is changed). + */ + if ( p2m_is_valid(orig_pte) && + !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) ) + p2m_free_entry(p2m, orig_pte, level); + out: unmap_domain_page(table);