Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1721670

Re: [PATCH v2 4/5] cramfs: add mmap support

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: [PATCH v2 4/5] cramfs: add mmap support
Date 2017-08-28 16:30 +0200
Message-ID <ujtd9-6G2-43@gated-at.bofh.it> (permalink)
References <ufasq-1zg-11@gated-at.bofh.it> <ufasq-1zg-9@gated-at.bofh.it> <ujm1Y-2ab-15@gated-at.bofh.it> <ujsqL-6aY-33@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Aug 28, 2017 at 09:29:58AM -0400, Nicolas Pitre wrote:
> > > +	/* Make sure the vma didn't change between the locks */
> > > +	vma = find_vma(mm, vmf->address);
> > > +	if (vma->vm_ops != &cramfs_vmasplit_ops) {
> > > +		/*
> > > +		 * Someone else raced with us and could have handled the fault.
> > > +		 * Let it go back to user space and fault again if necessary.
> > > +		 */
> > > +		downgrade_write(&mm->mmap_sem);
> > > +		return VM_FAULT_NOPAGE;
> > > +	}
> > > +
> > > +	/* Split the vma between the directly mapped area and the rest */
> > > +	ret = split_vma(mm, vma, split_addr, 0);
> > 
> > Egads...  Everything else aside, who said that your split_... will have
> > anything to do with the vma you get from find_vma()?
> 
> When vma->vm_ops == &cramfs_vmasplit_ops it is guaranteed that the vma 
> is not fully populated and that the unpopulated area starts at 
> split_addr. That split_addr was stored in vma->vm_private_data at the 
> same time as vma->vm_ops. Given that mm->mmap_sem is held all along 
> across find_vma(), split_vma() and the second find_vma() I hope that I 
> can trust that things will be related.

Huh?  You do realize that another thread might've been blocked on that ->mmap_sem
in mremap(), get it, have ours block on attempt to get ->mmap_sem exclusive,
exterminate the original vma and put there a vma that has also come from cramfs,
but other than that had not a damn thing in common with the original.  Different
memory area, etc.

Matching ->vm_ops is nowhere near enough.

While we are at it, what happens if you mmap 120Kb, then munmap() the middle
40Kb.  Leaving two 40Kb VMAs with 40Kb gap between them, that is.  Will your
->vm_private_data be correct for both?

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2 4/5] cramfs: add mmap support Al Viro <viro@ZenIV.linux.org.uk> - 2017-08-28 08:50 +0200
  Re: [PATCH v2 4/5] cramfs: add mmap support Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-08-28 15:40 +0200
    Re: [PATCH v2 4/5] cramfs: add mmap support Al Viro <viro@ZenIV.linux.org.uk> - 2017-08-28 16:30 +0200
      Re: [PATCH v2 4/5] cramfs: add mmap support Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-08-28 21:20 +0200
        RE: [PATCH v2 4/5] cramfs: add mmap support Chris Brandt <Chris.Brandt@renesas.com> - 2017-08-29 21:40 +0200
          RE: [PATCH v2 4/5] cramfs: add mmap support Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-08-29 22:10 +0200
            RE: [PATCH v2 4/5] cramfs: add mmap support Chris Brandt <Chris.Brandt@renesas.com> - 2017-08-29 22:20 +0200
            RE: [PATCH v2 4/5] cramfs: add mmap support Chris Brandt <Chris.Brandt@renesas.com> - 2017-08-31 04:40 +0200

csiph-web