From patchwork Wed Nov 1 08:05:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhenhua Huang X-Patchwork-Id: 13442597 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 0A07EC4332F for ; Wed, 1 Nov 2023 08:06:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:Subject:CC :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=DZK+nmFX1xu5hrWPjglcw/SZ4rrWrI++PSBhad+RwsM=; b=WkgM0yu/QMeqrv mPpjw7GdpTFC63ATwURtaBWsXjcQiZt+nNGp1FYPsSHu6sjX/8TAuqovM7gNq0IRVqI1KEdGYGy4y RjVIeQGT4TJgxFtJYbm49z+qURSRIbdgSmI9I80a9Ts5IeeTs/2N1BAxUD9/Jio9S4n/AfMNhv8Oi yUGGkDFx2HUWCr1EuNT/g3cOxIfRQBY5ZPARhUIDGnYDtHT7bhi5PiK7NhY5Z9U+dSWmXoh5Z5NSu neaNMN37UPSme/Y1himaH7340DtMmasB6FkDz9H0ujqqth4cZRDkKYINa3KuQZEKV/PEiSlZi/tdA uwfC3XZzSm/t1VC3XASw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qy6ES-006rMl-1R; Wed, 01 Nov 2023 08:05:44 +0000 Received: from mx0a-0031df01.pphosted.com ([205.220.168.131]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qy6EM-006rLq-2s for linux-arm-kernel@lists.infradead.org; Wed, 01 Nov 2023 08:05:42 +0000 Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3A167FBa020921; Wed, 1 Nov 2023 08:05:20 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : mime-version : content-type; s=qcppdkim1; bh=k1Re8VxmaE+1U/WnoIwTJGtW8BEZm/Vip5onsT835yo=; b=QAOpCZvaPJlWbh5vCMe9BxeS3lfuK9ip47nlSQXlunyuM1U5OjZCfUaKKqpqA0w3rvBb dmFtKbUBQyVQWDtVBWlS17uc5yvqaSpfie6d4JU9gsRXHbKwrBbe2GlzBm3TY2Jfm2tg SbuXLFX3RrHiBTmHBnxHtz6R7OMSHjbCY4VIF4cYc3M2rqF/HNcOnks1p9/m1rzq3G79 g9hN6wn4eL5wJ2sTgHc+JrCLm4yOQMOKWgMNfRuH4cMeGTcmc/WpB0RfgfIsdvxBR2/I D9idgkQu6PH8vgMsqNccqbGVpQgMqq/MIY3+Hr0Oq0hHVNppC3n5aT8aUTK4Dlrnhx6E Hw== Received: from nalasppmta04.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3u30xeb8ue-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 01 Nov 2023 08:05:20 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3A185H1x008615 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 1 Nov 2023 08:05:19 GMT Received: from zhenhuah-gv.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.39; Wed, 1 Nov 2023 01:05:15 -0700 From: Zhenhua Huang To: , , , , CC: , , , Zhenhua Huang Subject: [RFC PATCH 1/1] iommu: Fix one concurrency issue happens on non-iommu binding device Date: Wed, 1 Nov 2023 16:05:02 +0800 Message-ID: <1698825902-10685-1-git-send-email-quic_zhenhuah@quicinc.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: _67YCWWv0T6Ki9B7MP6sTqTWSWo2DmV_ X-Proofpoint-ORIG-GUID: _67YCWWv0T6Ki9B7MP6sTqTWSWo2DmV_ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-01_05,2023-10-31_03,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 spamscore=0 clxscore=1011 mlxscore=0 priorityscore=1501 malwarescore=0 adultscore=0 impostorscore=0 mlxlogscore=731 phishscore=0 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2310240000 definitions=main-2311010067 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231101_010538_933452_1AD14D19 X-CRM114-Status: GOOD ( 24.14 ) 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 Thread (1) During SMMU probe: iommu_device_register bus_iommu_probe probe_iommu_group for all devices in bus _probe_iommu_device (non-iommu binding device) first allocate dev->iommu then free Thread (2) client device probing: dma_configure of_iommu_configure get dev->iommu->fwspec and call fwspec->ops There may be a time window that dev->iommu allocated even for non iommu binding device in thread (1). It would be possible thread (2) meet Use- after-free errors at that window. Fix it by closing the time window, set dev->iommu only once ops->probe_device successfully. Previous discussion details: https://lore.kernel.org/linux-arm-kernel/20231017163337.GE282036@ziepe.ca/T/#mee0d7bdc375541934a571ae69f43b9660f8e7312 Suggested-by: Jason Gunthorpe Signed-off-by: Zhenhua Huang --- Hi Jason, Robin, Shall we first address non-iommu binding device crash with below patch? It's seen a lot especially when smmu probing and other driver probing at same time. Although it doesn't comprehensively fixes race with of_xlate VS probe, it indeed solves issue for non-iommu binding device. drivers/iommu/iommu.c | 42 +++++++++++++++++++++++++++++------------- include/linux/iommu.h | 5 +---- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f17a111..88ce802 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -386,6 +386,17 @@ static u32 dev_iommu_get_max_pasids(struct device *dev) return min_t(u32, max_pasids, dev->iommu->iommu_dev->max_pasids); } +void dev_iommu_priv_set(struct device *dev, void *priv) +{ + struct dev_iommu *dev_iommu; + + dev_iommu = dev_iommu_get(dev); + if (WARN_ON(!dev_iommu)) + return; + + dev->iommu->priv = priv; +} + /* * Init the dev->iommu and dev->iommu_group in the struct device and get the * driver probed @@ -393,23 +404,26 @@ static u32 dev_iommu_get_max_pasids(struct device *dev) static int iommu_init_device(struct device *dev, const struct iommu_ops *ops) { struct iommu_device *iommu_dev; + struct dev_iommu *dev_iommu; struct iommu_group *group; int ret; - if (!dev_iommu_get(dev)) - return -ENOMEM; - - if (!try_module_get(ops->owner)) { - ret = -EINVAL; - goto err_free; - } + if (!try_module_get(ops->owner)) + return -EINVAL; iommu_dev = ops->probe_device(dev); if (IS_ERR(iommu_dev)) { ret = PTR_ERR(iommu_dev); goto err_module_put; } - dev->iommu->iommu_dev = iommu_dev; + + dev_iommu = dev_iommu_get(dev); + if (WARN_ON(!dev_iommu)) { + ret = -ENOMEM; + goto err_release; + } + + dev_iommu->iommu_dev = iommu_dev; ret = iommu_device_link(iommu_dev, dev); if (ret) @@ -424,9 +438,9 @@ static int iommu_init_device(struct device *dev, const struct iommu_ops *ops) } dev->iommu_group = group; - dev->iommu->max_pasids = dev_iommu_get_max_pasids(dev); + dev_iommu->max_pasids = dev_iommu_get_max_pasids(dev); if (ops->is_attach_deferred) - dev->iommu->attach_deferred = ops->is_attach_deferred(dev); + dev_iommu->attach_deferred = ops->is_attach_deferred(dev); return 0; err_unlink: @@ -436,9 +450,11 @@ static int iommu_init_device(struct device *dev, const struct iommu_ops *ops) ops->release_device(dev); err_module_put: module_put(ops->owner); -err_free: - dev->iommu->iommu_dev = NULL; - dev_iommu_free(dev); + /* + * If probe_device allocated a dev->iommu and things failed later + * we just leave it. We don't yet have robust locking, there + * could be concurrent users. + */ return ret; } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 73e58df..62e9c86 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -840,10 +840,7 @@ static inline void *dev_iommu_priv_get(struct device *dev) return NULL; } -static inline void dev_iommu_priv_set(struct device *dev, void *priv) -{ - dev->iommu->priv = priv; -} +void dev_iommu_priv_set(struct device *dev, void *priv); int iommu_probe_device(struct device *dev);