Re: [PATCH v1 08/10] iommu/arm-smmu-qcom: Merge table from arm-smmu-qcom-debug into match data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Patch [1] introduces a use after free bug in the function "qcom_smmu_create()" in file: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c wherein devm_krealloc() frees the old pointer marked by "smmu" but it is still being accessed later in qcom_smmu_impl_data() in the same function as:

qsmmu->cfg = qcom_smmu_impl_data(smmu);

The current patchset [2] implicitly fixes this issue as it doesn't access the freed ptr in the line:

qsmmu->cfg = data->cfg;

Hence, can this patchset[2] be propagated to branches where patchset[1] has been propagated already? The bug is currently present in all branches
that have patchset[1] but do not have patchset[2].

RFC:

This bug would be reintroduced if patchset [3] is accepted. This makes the path prone to such errors as it relies on the
developer's understanding on the internal implementation of devm_krealloc().

Hence, a better fix IMO, would be to remove the confusion around the freed "smmu" ptr in the following way:

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 549ae4dba3a6..6dd142ce75d1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -463,11 +463,12 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
        qsmmu = devm_krealloc(smmu->dev, smmu, sizeof(*qsmmu), GFP_KERNEL);
        if (!qsmmu)
                return ERR_PTR(-ENOMEM);
+       smmu = &qsmmu->smmu;

        qsmmu->smmu.impl = impl;
        qsmmu->cfg = data->cfg;

-       return &qsmmu->smmu;
+       return smmu;
 }

This is similar to the patch[4] which I've sent in-reply-to patch[3]. Will send a formal patch if you think this approach is better.

Please let me know your thoughts.

Thanks,
Pratyush


[1] https://lore.kernel.org/all/20220708094230.4349-1-quic_saipraka@xxxxxxxxxxx/ [2] https://lore.kernel.org/all/20221114170635.1406534-9-dmitry.baryshkov@xxxxxxxxxx/ [3] https://lore.kernel.org/all/20240201210529.7728-4-quic_c_gdjako@xxxxxxxxxxx/ [4] https://lore.kernel.org/all/20240213062608.13018-1-quic_pbrahma@xxxxxxxxxxx/




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux