From patchwork Fri Feb 28 15:46:30 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robin Murphy X-Patchwork-Id: 13996703 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2DBEEC282C1 for ; Fri, 28 Feb 2025 16:05:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=JAAcmnGCaGEzDc3cTFs74OOyYgqDXc6/JXC9hW3Cyy4=; b=p4ThbZ3vrETAKEbuht1Dobkgow LrikWPhEH3aJDh4XsgsVOLYhHqmtXerlN2rF3LtqUKJIa53ANkFTm5ww6bdbHOSz6YcLdiCzIvtYu dZAq6Rrx6kjlYCiLNrR6uy4Slgdy5E9GXdsYNej7iGg7p6HiKgWQqLUMcnijD6322X0AT1o6+kDhE fLKj5EuOxi+liY3BHS4wXL8pX0+zSBLYd7DnR48OvzR+KlZjn6DEcOdYNx/FgjhcUcUTmlOTI+XBN nbGIPciOmngNutsErLvRb/xC3SzZn8JrnaIRqPziK5QY/TgaYB8IwMdxw25bzoV+ei2bVRulL49ZA PA+fYDEQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1to2rR-0000000Bazy-1s4Q; Fri, 28 Feb 2025 16:05:13 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1to2Zf-0000000BWwH-1xQS for linux-arm-kernel@lists.infradead.org; Fri, 28 Feb 2025 15:46:52 +0000 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 2DAF41688; Fri, 28 Feb 2025 07:47:06 -0800 (PST) Received: from e121345-lin.cambridge.arm.com (e121345-lin.cambridge.arm.com [10.1.196.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 9C66D3F5A1; Fri, 28 Feb 2025 07:46:47 -0800 (PST) From: Robin Murphy To: Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , "Rafael J. Wysocki" , Len Brown , Russell King , Greg Kroah-Hartman , Danilo Krummrich , Stuart Yoder , Laurentiu Tudor , Nipun Gupta , Nikhil Agarwal , Joerg Roedel , Will Deacon , Rob Herring , Saravana Kannan , Bjorn Helgaas Cc: linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, iommu@lists.linux.dev, devicetree@vger.kernel.org, linux-pci@vger.kernel.org, Charan Teja Kalla , Jason Gunthorpe Subject: [PATCH v2 1/4] iommu: Handle race with default domain setup Date: Fri, 28 Feb 2025 15:46:30 +0000 Message-Id: X-Mailer: git-send-email 2.39.2.101.g768bb238c484.dirty In-Reply-To: References: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250228_074651_636053_BB845D2E X-CRM114-Status: GOOD ( 15.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org It turns out that deferred default domain creation leaves a subtle race window during iommu_device_register() wherein a client driver may asynchronously probe in parallel and get as far as performing DMA API operations with dma-direct, only to be switched to iommu-dma underfoot once the default domain attachment finally happens, with obviously disastrous consequences. Even the wonky of_iommu_configure() path is at risk, since iommu_fwspec_init() will no longer defer client probe as the instance ops are (necessarily) already registered, and the "replay" iommu_probe_device() call can see dev->iommu_group already set and so think there's nothing to do either. Fortunately we already have the right tool in the right place in the form of iommu_device_use_default_domain(), which just needs to ensure that said default domain is actually ready to *be* used. Deferring the client probe shouldn't have too much impact, given that this only happens while the IOMMU driver is probing, and thus due to kick the deferred probe list again once it finishes. Reported-by: Charan Teja Kalla Fixes: 98ac73f99bc4 ("iommu: Require a default_domain for all iommu drivers") Reviewed-by: Jason Gunthorpe Signed-off-by: Robin Murphy --- v2: No change Note this fixes tag is rather nuanced - historically there was a more general issue before deac0b3bed26 ("iommu: Split off default domain allocation from group assignment") set the basis for the current conditions; 1ea2a07a532b ("iommu: Add DMA ownership management interfaces") is then the point at which it becomes logical to fix the current race this way; however only from 98ac73f99bc4 can we rely on all drivers supporting default domains and so avoid false negatives, thus even though this might apply to older kernels without conflict it would not be functionally correct. LTS-wise, prior to 6.6 and commit f188056352bc ("iommu: Avoid locking/unlocking for iommu_probe_device()") the impact of this race is merely the historical issue again, but since deac0b3bed26 that would raise a visible warning if it did lead to a default domain mismatch, which nobody has ever reported seeing. Thus we should only need a backport for 6.6, which I tentatively have ready. --- drivers/iommu/iommu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 4fca10d107c8..179617bb412d 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3107,6 +3107,11 @@ int iommu_device_use_default_domain(struct device *dev) return 0; mutex_lock(&group->mutex); + /* We may race against bus_iommu_probe() finalising groups here */ + if (!group->default_domain) { + ret = -EPROBE_DEFER; + goto unlock_out; + } if (group->owner_cnt) { if (group->domain != group->default_domain || group->owner || !xa_empty(&group->pasid_array)) {