linux-ext4 - Re: [PATCH 17/18] dax: Use radix tree entry lock to protect cow faults

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: <20160419151904.GA17318@gmail.com>
Date:	Tue, 19 Apr 2016 11:19:04 -0400
From:	Jerome Glisse <j.glisse@...il.com>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
	linux-mm@...ck.org, Ross Zwisler <ross.zwisler@...ux.intel.com>,
	Dan Williams <dan.j.williams@...el.com>,
	linux-nvdimm@...ts.01.org, Matthew Wilcox <willy@...ux.intel.com>
Subject: Re: [PATCH 17/18] dax: Use radix tree entry lock to protect cow
 faults

On Tue, Apr 19, 2016 at 04:33:43PM +0200, Jan Kara wrote:
> On Tue 19-04-16 07:46:09, Jerome Glisse wrote:
> > On Mon, Apr 18, 2016 at 11:35:40PM +0200, Jan Kara wrote:
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 93897f23cc11..f09cdb8d48fa 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -63,6 +63,7 @@
> > >  #include <linux/dma-debug.h>
> > >  #include <linux/debugfs.h>
> > >  #include <linux/userfaultfd_k.h>
> > > +#include <linux/dax.h>
> > >  
> > >  #include <asm/io.h>
> > >  #include <asm/mmu_context.h>
> > > @@ -2785,7 +2786,8 @@ oom:
> > >   */
> > >  static int __do_fault(struct vm_area_struct *vma, unsigned long address,
> > >  			pgoff_t pgoff, unsigned int flags,
> > > -			struct page *cow_page, struct page **page)
> > > +			struct page *cow_page, struct page **page,
> > > +			void **entry)
> > >  {
> > >  	struct vm_fault vmf;
> > >  	int ret;
> > > @@ -2800,8 +2802,10 @@ static int __do_fault(struct vm_area_struct *vma, unsigned long address,
> > >  	ret = vma->vm_ops->fault(vma, &vmf);
> > >  	if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY)))
> > >  		return ret;
> > > -	if (!vmf.page)
> > > -		goto out;
> > 
> > Removing the above sounds seriously bogus to me as it means that below
> > if (unlikely(PageHWPoison(vmf.page))) could dereference a NULL pointer.
> 
> If you do not return a valid page, you must return appropriate return code
> from the ->fault handler. That being VM_FAULT_NOPAGE, VM_FAULT_DAX_LOCKED,
> or some error. That has always been the case except for DAX abuse which was
> added by commit 2e4cdab0584f "mm: allow page fault handlers to perform the
> COW" about an year ago. And my patch fixes this abuse.
> 
> I'm not aware of any other code that would start abusing the return value
> from the ->fault handler. If some such code indeed got merged during the
> last year, it should be fixed as well.
> 

Ok my bad i missed that.

Cheers,
Jérôme
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html