linux-ext4 - Re: [PATCH 6/7] dax: Implement dax_pfn

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: <20170727230440.GI22000@linux.intel.com>
Date:   Thu, 27 Jul 2017 17:04:40 -0600
From:   Ross Zwisler <ross.zwisler@...ux.intel.com>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>,
        Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org,
        linux-ext4@...r.kernel.org,
        Dan Williams <dan.j.williams@...el.com>,
        Andy Lutomirski <luto@...nel.org>, linux-nvdimm@...ts.01.org,
        linux-xfs@...r.kernel.org, Christoph Hellwig <hch@...radead.org>,
        Dave Chinner <david@...morbit.com>
Subject: Re: [PATCH 6/7] dax: Implement dax_pfn_mkwrite()

On Thu, Jul 27, 2017 at 04:53:22PM -0600, Ross Zwisler wrote:
> On Thu, Jul 27, 2017 at 03:12:44PM +0200, Jan Kara wrote:
> > Implement a function that marks existing page table entry (PTE or PMD)
> > as writeable and takes care of marking it dirty in the radix tree. This
> > function will be used to finish synchronous page fault where the page
> > table entry is first inserted as read-only and then needs to be marked
> > as read-write.
> > 
> > Signed-off-by: Jan Kara <jack@...e.cz>
<>
> > +	radix_tree_tag_set(&mapping->page_tree, index, PAGECACHE_TAG_DIRTY);
> > +	entry = lock_slot(mapping, slot);
> > +	spin_unlock_irq(&mapping->tree_lock);
> > +	switch (pe_size) {
> > +	case PE_SIZE_PTE:
> > +		error = vm_insert_mixed_mkwrite(vmf->vma, vmf->address, pfn);
> 
> This path goes through the 'mkwrite' branch in insert_pfn() where we validate
> that the PFN we are about to map matches the one pointed to by the existing
> PTE, but I don't see any checks in this path that validate against
> vmf->orig_pte?
> 
> This kind of check was done by the old
> dax_pfn_mkwrite()->finish_mkwrite_fault() path via the pte_same() check in
> finish_mkwrite_fault().
> 
> Do we need to add an equivalent check somewhere in this path, since we're
> going through the trouble of setting vmf->orig_pte in the DAX fault handlers?

Or, actually, does the fact that we do the PFN based sanity check in
insert_pfn() mean that we don't actually need to set vmf->orig_pte in the DAX
fault handlers at all?  We already sanity check the PFN of the RW PTE we
insert against what is already in the page table, and maybe we don't need to
keep another copy around in vmf->orig_pte to verify against?