dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>
Cc: linux-s390@vger.kernel.org, "Rik van Riel" <riel@redhat.com>,
	linux-samsung-soc@vger.kernel.org, "Jan Kara" <jack@suse.cz>,
	"Kees Cook" <keescook@chromium.org>,
	kvm@vger.kernel.org, "Jason Gunthorpe" <jgg@ziepe.ca>,
	"Dave Airlie" <airlied@linux.ie>,
	linux-mm@kvack.org, "Jérôme Glisse" <jglisse@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Hugh Dickins" <hugh@veritas.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 07/13] mm: close race in generic_access_phys
Date: Wed, 7 Oct 2020 17:44:46 -0700	[thread overview]
Message-ID: <852a74ec-339b-4c7f-9e29-b9736111849a@nvidia.com> (raw)
In-Reply-To: <20201007164426.1812530-8-daniel.vetter@ffwll.ch>

On 10/7/20 9:44 AM, Daniel Vetter wrote:
> Way back it was a reasonable assumptions that iomem mappings never
> change the pfn range they point at. But this has changed:
> 
> - gpu drivers dynamically manage their memory nowadays, invalidating
>    ptes with unmap_mapping_range when buffers get moved
> 
> - contiguous dma allocations have moved from dedicated carvetouts to

s/carvetouts/carveouts/

>    cma regions. This means if we miss the unmap the pfn might contain
>    pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
> 
> - even /dev/mem now invalidates mappings when the kernel requests that
>    iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87
>    ("/dev/mem: Revoke mappings when a driver claims the region")

Thanks for putting these references into the log, it's very helpful.
...
> diff --git a/mm/memory.c b/mm/memory.c
> index fcfc4ca36eba..8d467e23b44e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4873,28 +4873,68 @@ int follow_phys(struct vm_area_struct *vma,
>   	return ret;
>   }
>   
> +/**
> + * generic_access_phys - generic implementation for iomem mmap access
> + * @vma: the vma to access
> + * @addr: userspace addres, not relative offset within @vma
> + * @buf: buffer to read/write
> + * @len: length of transfer
> + * @write: set to FOLL_WRITE when writing, otherwise reading
> + *
> + * This is a generic implementation for &vm_operations_struct.access for an
> + * iomem mapping. This callback is used by access_process_vm() when the @vma is
> + * not page based.
> + */
>   int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>   			void *buf, int len, int write)
>   {
>   	resource_size_t phys_addr;
>   	unsigned long prot = 0;
>   	void __iomem *maddr;
> +	pte_t *ptep, pte;
> +	spinlock_t *ptl;
>   	int offset = addr & (PAGE_SIZE-1);
> +	int ret = -EINVAL;
> +
> +	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
> +		return -EINVAL;
> +
> +retry:
> +	if (follow_pte(vma->vm_mm, addr, &ptep, &ptl))
> +		return -EINVAL;
> +	pte = *ptep;
> +	pte_unmap_unlock(ptep, ptl);
>   
> -	if (follow_phys(vma, addr, write, &prot, &phys_addr))
> +	prot = pgprot_val(pte_pgprot(pte));
> +	phys_addr = (resource_size_t)pte_pfn(pte) << PAGE_SHIFT;
> +
> +	if ((write & FOLL_WRITE) && !pte_write(pte))
>   		return -EINVAL;
>   
>   	maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
>   	if (!maddr)
>   		return -ENOMEM;
>   
> +	if (follow_pte(vma->vm_mm, addr, &ptep, &ptl))
> +		goto out_unmap;
> +
> +	if (pte_same(pte, *ptep)) {


The ioremap area is something I'm sorta new to, so a newbie question:
is it possible for the same pte to already be there, ever? If so, we
be stuck in an infinite loop here.  I'm sure that's not the case, but
it's not yet obvious to me why it's impossible. Resource reservations
maybe?


thanks,
-- 
John Hubbard
NVIDIA
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-10-08  0:44 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 16:44 [PATCH 00/13] follow_pfn and other iomap races Daniel Vetter
2020-10-07 16:44 ` [PATCH 01/13] drm/exynos: Stop using frame_vector helpers Daniel Vetter
2020-10-07 20:32   ` John Hubbard
2020-10-07 21:32     ` Daniel Vetter
2020-10-07 21:36       ` John Hubbard
2020-10-07 21:50         ` Daniel Vetter
2020-10-07 16:44 ` [PATCH 02/13] drm/exynos: Use FOLL_LONGTERM for g2d cmdlists Daniel Vetter
2020-10-07 20:43   ` John Hubbard
2020-10-07 16:44 ` [PATCH 03/13] misc/habana: Stop using frame_vector helpers Daniel Vetter
2020-10-07 20:38   ` John Hubbard
2020-10-07 16:44 ` [PATCH 04/13] misc/habana: Use FOLL_LONGTERM for userptr Daniel Vetter
2020-10-07 20:46   ` John Hubbard
2020-10-07 16:44 ` [PATCH 05/13] mm/frame-vector: Use FOLL_LONGTERM Daniel Vetter
2020-10-07 16:53   ` Jason Gunthorpe
2020-10-07 17:12     ` Daniel Vetter
2020-10-07 17:33       ` Jason Gunthorpe
2020-10-07 21:13   ` John Hubbard
2020-10-07 21:30     ` Daniel Vetter
2020-10-07 16:44 ` [PATCH 06/13] media: videobuf2: Move frame_vector into media subsystem Daniel Vetter
2020-10-07 22:18   ` John Hubbard
2020-10-07 16:44 ` [PATCH 07/13] mm: close race in generic_access_phys Daniel Vetter
2020-10-07 17:27   ` Jason Gunthorpe
2020-10-07 18:01     ` Daniel Vetter
2020-10-07 23:21       ` Jason Gunthorpe
2020-10-08  0:44   ` John Hubbard [this message]
2020-10-08  7:23     ` Daniel Vetter
2020-10-07 16:44 ` [PATCH 08/13] s390/pci: Remove races against pte updates Daniel Vetter
2020-10-08 16:44   ` Gerald Schaefer
2020-10-08 17:16     ` Daniel Vetter
2020-10-07 16:44 ` [PATCH 09/13] PCI: obey iomem restrictions for procfs mmap Daniel Vetter
2020-10-07 18:46   ` Bjorn Helgaas
2020-10-07 16:44 ` [PATCH 10/13] PCI: revoke mappings like devmem Daniel Vetter
2020-10-07 18:41   ` Bjorn Helgaas
2020-10-07 19:24     ` Daniel Vetter
2020-10-07 19:33   ` Dan Williams
2020-10-07 19:47     ` Daniel Vetter
2020-10-07 22:23       ` Dan Williams
2020-10-07 22:29         ` Dan Williams
2020-10-08  8:09           ` Daniel Vetter
2020-10-07 23:24     ` Jason Gunthorpe
2020-10-08  7:31       ` Daniel Vetter
2020-10-08  7:49       ` Dan Williams
2020-10-08  8:13         ` Daniel Vetter
2020-10-08  8:35           ` Dan Williams
2020-10-08 12:41         ` Jason Gunthorpe
2020-10-07 16:44 ` [PATCH 11/13] mm: add unsafe_follow_pfn Daniel Vetter
2020-10-07 17:36   ` Jason Gunthorpe
2020-10-07 18:10     ` Daniel Vetter
2020-10-07 19:00       ` Jason Gunthorpe
2020-10-07 19:38         ` Daniel Vetter
2020-10-07 16:44 ` [PATCH 12/13] media/videbuf1|2: Mark follow_pfn usage as unsafe Daniel Vetter
2020-10-07 16:44 ` [PATCH 13/13] vfio/type1: Mark follow_pfn " Daniel Vetter
2020-10-07 17:39   ` Jason Gunthorpe
2020-10-07 18:14     ` Daniel Vetter
2020-10-07 18:47       ` Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=852a74ec-339b-4c7f-9e29-b9736111849a@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=airlied@linux.ie \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hugh@veritas.com \
    --cc=jack@suse.cz \
    --cc=jgg@ziepe.ca \
    --cc=jglisse@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=riel@redhat.com \
    --subject='Re: [PATCH 07/13] mm: close race in generic_access_phys' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).