dax: fix data corruption when fault races with write
Currently DAX read fault can race with write(2) in the following way:
CPU1 - write(2) CPU2 - read fault
dax_iomap_pte_fault()
->iomap_begin() - sees hole
dax_iomap_rw()
iomap_apply()
->iomap_begin - allocates blocks
dax_iomap_actor()
invalidate_inode_pages2_range()
- there's nothing to invalidate
grab_mapping_entry()
- we add zero page in the radix tree
and map it to page tables
The result is that hole page is mapped into page tables (and thus zeros
are seen in mmap) while file has data written in that place.
Fix the problem by locking exception entry before mapping blocks for the
fault. That way we are sure invalidate_inode_pages2_range() call for
racing write will either block on entry lock waiting for the fault to
finish (and unmap stale page tables after that) or read fault will see
already allocated blocks by write(2).
Fixes: 9f141d6ef6
Link: http://lkml.kernel.org/r/20170510085419.27601-5-jack@suse.cz
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
fb26a1cbed
commit
13e451fdc1
32
fs/dax.c
32
fs/dax.c
|
@ -1148,6 +1148,12 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
if ((vmf->flags & FAULT_FLAG_WRITE) && !vmf->cow_page)
|
if ((vmf->flags & FAULT_FLAG_WRITE) && !vmf->cow_page)
|
||||||
flags |= IOMAP_WRITE;
|
flags |= IOMAP_WRITE;
|
||||||
|
|
||||||
|
entry = grab_mapping_entry(mapping, vmf->pgoff, 0);
|
||||||
|
if (IS_ERR(entry)) {
|
||||||
|
vmf_ret = dax_fault_return(PTR_ERR(entry));
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Note that we don't bother to use iomap_apply here: DAX required
|
* Note that we don't bother to use iomap_apply here: DAX required
|
||||||
* the file system block size to be equal the page size, which means
|
* the file system block size to be equal the page size, which means
|
||||||
|
@ -1156,17 +1162,11 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
error = ops->iomap_begin(inode, pos, PAGE_SIZE, flags, &iomap);
|
error = ops->iomap_begin(inode, pos, PAGE_SIZE, flags, &iomap);
|
||||||
if (error) {
|
if (error) {
|
||||||
vmf_ret = dax_fault_return(error);
|
vmf_ret = dax_fault_return(error);
|
||||||
goto out;
|
goto unlock_entry;
|
||||||
}
|
}
|
||||||
if (WARN_ON_ONCE(iomap.offset + iomap.length < pos + PAGE_SIZE)) {
|
if (WARN_ON_ONCE(iomap.offset + iomap.length < pos + PAGE_SIZE)) {
|
||||||
vmf_ret = dax_fault_return(-EIO); /* fs corruption? */
|
error = -EIO; /* fs corruption? */
|
||||||
goto finish_iomap;
|
goto error_finish_iomap;
|
||||||
}
|
|
||||||
|
|
||||||
entry = grab_mapping_entry(mapping, vmf->pgoff, 0);
|
|
||||||
if (IS_ERR(entry)) {
|
|
||||||
vmf_ret = dax_fault_return(PTR_ERR(entry));
|
|
||||||
goto finish_iomap;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
sector = dax_iomap_sector(&iomap, pos);
|
sector = dax_iomap_sector(&iomap, pos);
|
||||||
|
@ -1188,13 +1188,13 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
}
|
}
|
||||||
|
|
||||||
if (error)
|
if (error)
|
||||||
goto error_unlock_entry;
|
goto error_finish_iomap;
|
||||||
|
|
||||||
__SetPageUptodate(vmf->cow_page);
|
__SetPageUptodate(vmf->cow_page);
|
||||||
vmf_ret = finish_fault(vmf);
|
vmf_ret = finish_fault(vmf);
|
||||||
if (!vmf_ret)
|
if (!vmf_ret)
|
||||||
vmf_ret = VM_FAULT_DONE_COW;
|
vmf_ret = VM_FAULT_DONE_COW;
|
||||||
goto unlock_entry;
|
goto finish_iomap;
|
||||||
}
|
}
|
||||||
|
|
||||||
switch (iomap.type) {
|
switch (iomap.type) {
|
||||||
|
@ -1214,7 +1214,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
case IOMAP_HOLE:
|
case IOMAP_HOLE:
|
||||||
if (!(vmf->flags & FAULT_FLAG_WRITE)) {
|
if (!(vmf->flags & FAULT_FLAG_WRITE)) {
|
||||||
vmf_ret = dax_load_hole(mapping, &entry, vmf);
|
vmf_ret = dax_load_hole(mapping, &entry, vmf);
|
||||||
goto unlock_entry;
|
goto finish_iomap;
|
||||||
}
|
}
|
||||||
/*FALLTHRU*/
|
/*FALLTHRU*/
|
||||||
default:
|
default:
|
||||||
|
@ -1223,10 +1223,8 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
error_unlock_entry:
|
error_finish_iomap:
|
||||||
vmf_ret = dax_fault_return(error) | major;
|
vmf_ret = dax_fault_return(error) | major;
|
||||||
unlock_entry:
|
|
||||||
put_locked_mapping_entry(mapping, vmf->pgoff, entry);
|
|
||||||
finish_iomap:
|
finish_iomap:
|
||||||
if (ops->iomap_end) {
|
if (ops->iomap_end) {
|
||||||
int copied = PAGE_SIZE;
|
int copied = PAGE_SIZE;
|
||||||
|
@ -1241,7 +1239,9 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
|
||||||
*/
|
*/
|
||||||
ops->iomap_end(inode, pos, PAGE_SIZE, copied, flags, &iomap);
|
ops->iomap_end(inode, pos, PAGE_SIZE, copied, flags, &iomap);
|
||||||
}
|
}
|
||||||
out:
|
unlock_entry:
|
||||||
|
put_locked_mapping_entry(mapping, vmf->pgoff, entry);
|
||||||
|
out:
|
||||||
trace_dax_pte_fault_done(inode, vmf, vmf_ret);
|
trace_dax_pte_fault_done(inode, vmf, vmf_ret);
|
||||||
return vmf_ret;
|
return vmf_ret;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue