diff mbox

dax: fix PMD faults on zero-length files

Message ID x49y3n8s4o8.fsf@segfault.boston.devel.redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Moyer Nov. 15, 2017, 1:37 a.m. UTC
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>

Comments

Dan Williams Nov. 15, 2017, 1:47 a.m. UTC | #1
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.
Ross Zwisler Nov. 15, 2017, 7:16 p.m. UTC | #2
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 mbox

Patch

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;
 
 	/*