linux-kernel - Re: [PATCH v5 6/9] iommu/dma-iommu.c: Convert to use vm_insert

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] [day] [month] [year] [list]

Message-ID: <20181224142927.GZ26090@n2100.armlinux.org.uk>
Date:   Mon, 24 Dec 2018 14:29:27 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Souptick Joarder <jrdr.linux@...il.com>
Cc:     akpm@...ux-foundation.org, willy@...radead.org, mhocko@...e.com,
        joro@...tes.org, robin.murphy@....com,
        iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v5 6/9] iommu/dma-iommu.c: Convert to use vm_insert_range

On Mon, Dec 24, 2018 at 06:55:31PM +0530, Souptick Joarder wrote:
> Convert to use vm_insert_range() to map range of kernel
> memory to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@...il.com>
> Reviewed-by: Matthew Wilcox <willy@...radead.org>
> ---
>  drivers/iommu/dma-iommu.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index d1b0475..de7ffd8 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -622,17 +622,10 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
>  
>  int iommu_dma_mmap(struct page **pages, size_t size, struct vm_area_struct *vma)
>  {
> -	unsigned long uaddr = vma->vm_start;
> -	unsigned int i, count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> -	int ret = -ENXIO;
> +	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>  
> -	for (i = vma->vm_pgoff; i < count && uaddr < vma->vm_end; i++) {
> -		ret = vm_insert_page(vma, uaddr, pages[i]);
> -		if (ret)
> -			break;
> -		uaddr += PAGE_SIZE;
> -	}
> -	return ret;
> +	return vm_insert_range(vma, vma->vm_start, pages + vma->vm_pgoff,
> +				count - vma->vm_pgoff);

This introduces a new bug.

I'm not going to continue to point out in minute detail the mistakes
you are introducing, as I don't think that is helping you to learn.

Look at this closely, and see whether you can spot the mistake.
Specifically, compare the boundary conditions for the final page
that is to be inserted and the value returned by the original version
and by your version for different scenarios.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up