linux-kernel - Re: [PATCH v3 4/5] iommu/uapi: Handle data and argsz filled by users

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]

Date:   Thu, 25 Jun 2020 15:07:27 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Jacob Pan <jacob.jun.pan@...ux.intel.com>
Cc:     baolu.lu@...ux.intel.com, iommu@...ts.linux-foundation.org,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Yi Liu <yi.l.liu@...el.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        Raj Ashok <ashok.raj@...el.com>,
        Christoph Hellwig <hch@...radead.org>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        Eric Auger <eric.auger@...hat.com>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v3 4/5] iommu/uapi: Handle data and argsz filled by users

On 2020/6/25 1:07, Jacob Pan wrote:
> On Wed, 24 Jun 2020 14:54:49 +0800
> Lu Baolu<baolu.lu@...ux.intel.com>  wrote:
> 
>> Hi Jacob,
>>
>> On 2020/6/24 1:03, Jacob Pan wrote:
>>> IOMMU UAPI data has a user filled argsz field which indicates the
>>> data length comes with the API call. User data is not trusted,
>>> argsz must be validated based on the current kernel data size,
>>> mandatory data size, and feature flags.
>>>
>>> User data may also be extended, results in possible argsz increase.
>>> Backward compatibility is ensured based on size and flags checking.
>>> Details are documented in Documentation/userspace-api/iommu.rst
>>>
>>> This patch adds sanity checks in both IOMMU layer and vendor code,
>>> where VT-d is the only user for now.
>>>
>>> Signed-off-by: Liu Yi L<yi.l.liu@...el.com>
>>> Signed-off-by: Jacob Pan<jacob.jun.pan@...ux.intel.com>
>>> ---
>>>    drivers/iommu/intel/svm.c |  3 ++
>>>    drivers/iommu/iommu.c     | 96
>>> ++++++++++++++++++++++++++++++++++++++++++++---
>>> include/linux/iommu.h     |  7 ++-- 3 files changed, 98
>>> insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
>>> index 713b3a218483..237db56878c0 100644
>>> --- a/drivers/iommu/intel/svm.c
>>> +++ b/drivers/iommu/intel/svm.c
>>> @@ -244,6 +244,9 @@ int intel_svm_bind_gpasid(struct iommu_domain
>>> *domain, struct device *dev, data->format !=
>>> IOMMU_PASID_FORMAT_INTEL_VTD) return -EINVAL;
>>>    
>>> +	if (data->argsz != offsetofend(struct
>>> iommu_gpasid_bind_data, vendor.vtd))
>>> +		return -EINVAL;
>> Need to do size check in intel_iommu_sva_invalidate() as well?
>>
> No need. The difference is that there is no
> vendor specific union for intel_iommu_sva_invalidate().
> 
> Generic flags are used to process invalidation data inside
> intel_iommu_sva_invalidate().

Thanks for the explanation. With the nit tweaked,

Reviewed-by: Lu Baolu <baolu.lu@...ux.intel.com>

Best regards,
baolu