| Message ID | x49y3n8s4o8.fsf@segfault.boston.devel.redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 957ac8c421ad |
| Headers | show |
On Tue, Nov 14, 2017 at 5:37 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > PMD faults on a zero length file on a file system mounted with -o dax > will not generate SIGBUS as expected. > > fd = open(...O_TRUNC); > addr = mmap(NULL, 2*1024*1024, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); > *addr = 'a'; > <expect SIGBUS> > > The problem is this code in dax_iomap_pmd_fault: > > max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT; > > If the inode size is zero, we end up with a max_pgoff that is way larger > than 0. :) Fix it by using DIV_ROUND_UP, as is done elsewhere in the > kernel. > > I tested this with some simple test code that ensured that SIGBUS was > received where expected. > I assume this needs: Fixes: 642261ac995e ("dax: add struct iomap based DAX PMD support") Cc: <stable@vger.kernel.org> ...otherwise looks good to me.
On Tue, Nov 14, 2017 at 05:47:51PM -0800, Dan Williams wrote: > On Tue, Nov 14, 2017 at 5:37 PM, Jeff Moyer <jmoyer@redhat.com> wrote: > > PMD faults on a zero length file on a file system mounted with -o dax > > will not generate SIGBUS as expected. > > > > fd = open(...O_TRUNC); > > addr = mmap(NULL, 2*1024*1024, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); > > *addr = 'a'; > > <expect SIGBUS> > > > > The problem is this code in dax_iomap_pmd_fault: > > > > max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT; > > > > If the inode size is zero, we end up with a max_pgoff that is way larger > > than 0. :) Fix it by using DIV_ROUND_UP, as is done elsewhere in the > > kernel. > > > > I tested this with some simple test code that ensured that SIGBUS was > > received where expected. > > > > I assume this needs: > > Fixes: 642261ac995e ("dax: add struct iomap based DAX PMD support") > Cc: <stable@vger.kernel.org> > > ...otherwise looks good to me. Yep, this fix looks good, thanks. You can add: Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
diff --git a/fs/dax.c b/fs/dax.c index f001d8c7..191306c 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -1327,7 +1327,7 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf, * this is a reliable test. */ pgoff = linear_page_index(vma, pmd_addr); - max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT; + max_pgoff = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE); trace_dax_pmd_fault(inode, vmf, max_pgoff, 0); @@ -1351,13 +1351,13 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf, if ((pmd_addr + PMD_SIZE) > vma->vm_end) goto fallback; - if (pgoff > max_pgoff) { + if (pgoff >= max_pgoff) { result = VM_FAULT_SIGBUS; goto out; } /* If the PMD would extend beyond the file size */ - if ((pgoff | PG_PMD_COLOUR) > max_pgoff) + if ((pgoff | PG_PMD_COLOUR) >= max_pgoff) goto fallback; /*
PMD faults on a zero length file on a file system mounted with -o dax will not generate SIGBUS as expected. fd = open(...O_TRUNC); addr = mmap(NULL, 2*1024*1024, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); *addr = 'a'; <expect SIGBUS> The problem is this code in dax_iomap_pmd_fault: max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT; If the inode size is zero, we end up with a max_pgoff that is way larger than 0. :) Fix it by using DIV_ROUND_UP, as is done elsewhere in the kernel. I tested this with some simple test code that ensured that SIGBUS was received where expected. Signed-off-by: Jeff Moyer <jmoyer@redhat.com>