2014-01-09 01:59:30

by Ethan Zhao

[permalink] [raw]
Subject: [PATCH Trivial] intel-iommu: check return value of device_to_iommu() before using it

in function iommu_support_dev_iotlb(),return value of device_to_iommu()
is used without checking, this could cause NULL pointer issue.

this patch is for v3.12.6

Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/iommu/intel-iommu.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 40203ad..5b4531a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1105,6 +1105,8 @@ static struct device_domain_info
*iommu_support_dev_iotlb(
struct device_domain_info *info;
struct intel_iommu *iommu = device_to_iommu(segment, bus, devfn);

+ if (!iommu)
+ return NULL;
if (!ecap_dev_iotlb_support(iommu->ecap))
return NULL;

--
1.7.1


Attachments:
0001-intel-iommu-check-return-value-of-device_to_iommu-be.patch (0.98 kB)

2014-01-09 12:01:46

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH Trivial] intel-iommu: check return value of device_to_iommu() before using it

On Thu, Jan 09, 2014 at 09:58:42AM +0800, ethan zhao wrote:
> drivers/iommu/intel-iommu.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 40203ad..5b4531a 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -1105,6 +1105,8 @@ static struct device_domain_info
> *iommu_support_dev_iotlb(
> struct device_domain_info *info;
> struct intel_iommu *iommu = device_to_iommu(segment, bus, devfn);
>
> + if (!iommu)
> + return NULL;
> if (!ecap_dev_iotlb_support(iommu->ecap))
> return NULL;

This call can not fail, iommu_support_dev_iotlb() is only called from
domain_context_mapping_one() which does the iommu==NULL check on its
own before the call.


Joerg

2014-01-10 02:16:35

by ethan zhao

[permalink] [raw]
Subject: Re: [PATCH Trivial] intel-iommu: check return value of device_to_iommu() before using it

On Thu, Jan 9, 2014 at 8:01 PM, Joerg Roedel <[email protected]> wrote:
> On Thu, Jan 09, 2014 at 09:58:42AM +0800, ethan zhao wrote:
>> drivers/iommu/intel-iommu.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 40203ad..5b4531a 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -1105,6 +1105,8 @@ static struct device_domain_info
>> *iommu_support_dev_iotlb(
>> struct device_domain_info *info;
>> struct intel_iommu *iommu = device_to_iommu(segment, bus, devfn);
>>
>> + if (!iommu)
>> + return NULL;
>> if (!ecap_dev_iotlb_support(iommu->ecap))
>> return NULL;
>
> This call can not fail, iommu_support_dev_iotlb() is only called from
> domain_context_mapping_one() which does the iommu==NULL check on its
> own before the call.
>
Agree, till now, was checked in domain_context_mapping_one(), but how
about if we query information with iommu_support_dev_iotlb() in other
code ?

Thanks,
Ethan
>
> Joerg
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html