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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <13da9cd9-218d-4b3f-98f8-62edcd91a23e@quicinc.com>
Date: Mon, 12 Feb 2024 22:59:50 +0530
From: Pratyush Brahma <quic_pbrahma@...cinc.com>
To: <quic_c_gdjako@...cinc.com>
CC: <andersson@...nel.org>, <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>, <djakov@...nel.org>,
        <iommu@...ts.linux.dev>, <joro@...tes.org>, <konrad.dybcio@...aro.org>,
        <krzysztof.kozlowski+dt@...aro.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <quic_cgoldswo@...cinc.com>, <quic_pdaly@...cinc.com>,
        <quic_sudaraja@...cinc.com>, <quic_sukadev@...cinc.com>,
        <robdclark@...il.com>, <robh+dt@...nel.org>, <robin.murphy@....com>,
        <will@...nel.org>
Subject: Re: [PATCH v4 03/10] iommu/arm-smmu-qcom: Add support for TBUs

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