Received-SPF: pass (google.com: domain of linux-kernel+bounces-62100-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <13da9cd9-218d-4b3f-98f8-62edcd91a23e@quicinc.com>
Date: Mon, 12 Feb 2024 22:59:50 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Pratyush Brahma <quic_pbrahma@quicinc.com>
Subject: Re: [PATCH v4 03/10] iommu/arm-smmu-qcom: Add support for TBUs
To: <quic_c_gdjako@quicinc.com>
CC: <andersson@kernel.org>, <conor+dt@kernel.org>,
        <devicetree@vger.kernel.org>, <djakov@kernel.org>,
        <iommu@lists.linux.dev>, <joro@8bytes.org>, <konrad.dybcio@linaro.org>,
        <krzysztof.kozlowski+dt@linaro.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_cgoldswo@quicinc.com>, <quic_pdaly@quicinc.com>,
        <quic_sudaraja@quicinc.com>, <quic_sukadev@quicinc.com>,
        <robdclark@gmail.com>, <robh+dt@kernel.org>, <robin.murphy@arm.com>,
        <will@kernel.org>
References: <20240201210529.7728-4-quic_c_gdjako@quicinc.com>
Content-Language: en-US
In-Reply-To: <20240201210529.7728-4-quic_c_gdjako@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi

The following patch would introduce a use-after-free bug which was found 
during KASAN testing on qcm6490 with the patch.

diff 
<https://lore.kernel.org/all/20240201210529.7728-4-quic_c_gdjako@quicinc.com/#iZ2e.:20240201210529.7728-4-quic_c_gdjako::40quicinc.com:1drivers:iommu:arm:arm-smmu:arm-smmu-qcom.c> 
--git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 
8b04ece00420..ca806644e6eb 100644 --- 
a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -1,12 +1,14 @@   // SPDX-License-Identifier: GPL-2.0-only
  /*
   * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved    */
  
  #include <linux/acpi.h>
  #include <linux/adreno-smmu-priv.h>
  #include <linux/delay.h>
  #include <linux/of_device.h>
+#include <linux/of_platform.h>   #include <linux/firmware/qcom/qcom_scm.h>
  
  #include "arm-smmu.h"
@@ -446,6 +448,7 @@ static struct arm_smmu_device 
*qcom_smmu_create(struct arm_smmu_device *smmu,   	const struct device_node *np = smmu->dev->of_node;
  	const struct arm_smmu_impl *impl;
  	struct qcom_smmu *qsmmu;
+ int ret;   
  	if (!data)
  		return ERR_PTR(-EINVAL);
@@ -469,6 +472,12 @@ static struct arm_smmu_device 
*qcom_smmu_create(struct arm_smmu_device *smmu,   	qsmmu->smmu.impl = impl;
  	qsmmu->cfg = data->cfg;
  
+ INIT_LIST_HEAD(&qsmmu->tbu_list); + mutex_init(&qsmmu->tbu_list_lock); 
+ ret = devm_of_platform_populate(smmu->dev); // smmu has been freed by 
devm_krealloc() above but is being accessed here again later. This 
causes use-after-free bug. + if (ret) + return ERR_PTR(ret); +   	return &qsmmu->smmu;
  }

Can it be done like below?
  	qsmmu->smmu.impl = impl;
  	qsmmu->cfg = data->cfg;
  
+ INIT_LIST_HEAD(&qsmmu->tbu_list); + mutex_init(&qsmmu->tbu_list_lock); 
+ ret = devm_of_platform_populate(qsmmu->smmu.dev);// Using the struct 
to which smmu was copied instead of freed ptr. Thanks, Pratyush