Re: [PATCH v4 03/10] iommu/arm-smmu-qcom: Add support for TBUs

[Pratyush Brahma <quic_pbrahma@quicinc.com>]

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