Re: [PATCH v3] fs: Fix page_mkwrite off-by-one errors

Matthew Wilcox
2019-12-18T19:21:55Z

On Wed, Dec 18, 2019 at 10:52:16AM -0800, Darrick J. Wong wrote:
> > @@ -9016,13 +9016,11 @@ vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf)
> > ret = VM_FAULT_NOPAGE; /* make the VM retry the fault */
> > again:
> > lock_page(page);
> > - size = i_size_read(inode);
> >
> > - if ((page->mapping != inode->i_mapping) ||
> > - (page_start >= size)) {
> > - /* page got truncated out from underneath us */
> > + ret2 = page_mkwrite_check_truncate(page, inode);
> > + if (ret2 < 0)
> > goto out_unlock;
>
> ...here we try to return -EFAULT as vm_fault_t. Notice how btrfs returns
> VM_FAULT_* values directly and never calls block_page_mkwrite_return? I
> know dsterba acked this, but I cannot see how this is correct?

I think you misread it. 'ret2' is never returned; we'll end up returning
VM_FAULT_NOPAGE here. Arguably it should be SIGBUS or something, but
I think retrying the fault will also end up giving a SIGBUS.