Absolutely. I already verified the bug on wrapfs first (easier to test in a smaller template :-), then fixed it there; then did the same to unionfs. Then back-ported to all previous versions applicable (some older kernels took a struct page as 2nd arg of ->page_mkwrite). And updated to latest kernels while I’m at it. All of this was pushed to git.fsl.cs.sunysb.edu. See actual patch below for the latest Linus tree.
Cheers, Erez. On May 14, 2014, at 11:23 PM, Theodore Ts'o <[email protected]> wrote: > BTW, I assume it won't surprise you to discover that wrapfs has the > same problem, right? diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c index 28b6ce8..488a582 100644 (file) --- a/fs/unionfs/mmap.c +++ b/fs/unionfs/mmap.c @@ -65,6 +65,41 @@ static int unionfs_fault(struct vm_area_struct *vma, struct vm_fault *vmf) return err; } +static int unionfs_page_mkwrite(struct vm_area_struct *vma, + struct vm_fault *vmf) +{ + int err = 0; + struct file *file, *lower_file; + const struct vm_operations_struct *lower_vm_ops; + struct vm_area_struct lower_vma; + + BUG_ON(!vma); + memcpy(&lower_vma, vma, sizeof(struct vm_area_struct)); + file = lower_vma.vm_file; + lower_vm_ops = UNIONFS_F(file)->lower_vm_ops; + BUG_ON(!lower_vm_ops); + if (!lower_vm_ops->page_mkwrite) + goto out; + + lower_file = unionfs_lower_file(file); + BUG_ON(!lower_file); + /* + * XXX: vm_ops->page_mkwrite may be called in parallel. + * Because we have to resort to temporarily changing the + * vma->vm_file to point to the lower file, a concurrent + * invocation of unionfs_page_mkwrite could see a different + * value. In this workaround, we keep a different copy of the + * vma structure in our stack, so we never expose a different + * value of the vma->vm_file called to us, even temporarily. + * A better fix would be to change the calling semantics of + * ->page_mkwrite to take an explicit file pointer. + */ + lower_vma.vm_file = lower_file; + err = lower_vm_ops->page_mkwrite(&lower_vma, vmf); +out: + return err; +} + /* * XXX: the default address_space_ops for unionfs is empty. We cannot set * our inode->i_mapping->a_ops to NULL because too many code paths expect @@ -86,4 +121,5 @@ struct address_space_operations unionfs_dummy_aops = { struct vm_operations_struct unionfs_vm_ops = { .fault = unionfs_fault, + .page_mkwrite = unionfs_page_mkwrite, }; _______________________________________________ unionfs mailing list: http://unionfs.filesystems.org/ [email protected] http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
