linux-kernel - Re: [RFC v1 5/8] mshv: add paravirtualized IOMMU support

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: <20210810104658.5ir7cibalhy3roun@liuwe-devbox-debian-v2>
Date:   Tue, 10 Aug 2021 10:46:58 +0000
From:   Wei Liu <wei.liu@...nel.org>
To:     Praveen Kumar <kumarpraveen@...ux.microsoft.com>
Cc:     Wei Liu <wei.liu@...nel.org>,
        Linux on Hyper-V List <linux-hyperv@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        Linux Kernel List <linux-kernel@...r.kernel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Vineeth Pillai <viremana@...ux.microsoft.com>,
        Sunil Muthuswamy <sunilmut@...rosoft.com>,
        Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
        pasha.tatashin@...een.com, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>,
        "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>
Subject: Re: [RFC v1 5/8] mshv: add paravirtualized IOMMU support

On Wed, Aug 04, 2021 at 12:13:42PM +0530, Praveen Kumar wrote:
> On 04-08-2021 03:17, Wei Liu wrote:
> >>> +static size_t hv_iommu_unmap(struct iommu_domain *d, unsigned long iova,
> >>> +			   size_t size, struct iommu_iotlb_gather *gather)
> >>> +{
> >>> +	size_t unmapped;
> >>> +	struct hv_iommu_domain *domain = to_hv_iommu_domain(d);
> >>> +	unsigned long flags, npages;
> >>> +	struct hv_input_unmap_device_gpa_pages *input;
> >>> +	u64 status;
> >>> +
> >>> +	unmapped = hv_iommu_del_mappings(domain, iova, size);
> >>> +	if (unmapped < size)
> >>> +		return 0;
> >> Is there a case where unmapped > 0 && unmapped < size ?
> >>
> > There could be such a case -- hv_iommu_del_mappings' return value is >= 0.
> > Is there a problem with this predicate?
> 
> What I understand, if we are unmapping and return 0, means nothing was
> unmapped, and will that not cause any corruption or illegal access of
> unmapped memory later?  From __iommu_unmap

Those pages are not really unmapped. The hypercall is skipped.

> ...
>     13         while (unmapped < size) {
>     12                 size_t pgsize = iommu_pgsize(domain, iova, size - unmapped);
>     11
>     10                 unmapped_page = ops->unmap(domain, iova, pgsize, iotlb_gather);
>      9                 if (!unmapped_page)
>      8                         break;		<<< we just break here, thinking there is nothing unmapped, but actually hv_iommu_del_mappings has removed some pages.
>      7
>      6                 pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
>      5                         ¦iova, unmapped_page);
>      4
>      3                 iova += unmapped_page;
>      2                 unmapped += unmapped_page;
>      1         }
> ...
> 
> Am I missing something ?
> 
> Regards,
> 
> ~Praveen.