| Message ID | 20240201210529.7728-4-quic_c_gdjako@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Translation Buffer Units | expand |
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
diff --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); + if (ret) + return ERR_PTR(ret); + return &qsmmu->smmu; } diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h index 593910567b88..77e5becc2482 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2024, Qualcomm Innovation Center, Inc. All rights reserved. */ #ifndef _ARM_SMMU_QCOM_H @@ -12,6 +12,8 @@ struct qcom_smmu { bool bypass_quirk; u8 bypass_cbndx; u32 stall_enabled; + struct mutex tbu_list_lock; /* protects tbu_list */ + struct list_head tbu_list; }; enum qcom_smmu_impl_reg_offset {
The ARM MMU-500 implements a Translation Buffer Unit (TBU) for each connected master besides a single TCU which controls and manages the address translations. Allow the Qualcomm SMMU driver to probe for any TBU devices that can provide additional debug features like triggering transactions, logging outstanding transactions, snapshot capture etc. The primary use-case would be to get information from a TBU and print it during a context fault. Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++ drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-)