On Tue, Dec 5, 2017 at 10:12 PM, Chad Austin <chadaustin@xxxxxx> wrote:
> When the atomic_o_trunc flag is set, fuse_do_setattr skips most of its
> logic, including truncating the page cache. Run the same invalidation
> logic in fuse_finish_open when truncating. This fixes a bug where
> open(O_TRUNC) followed by a sequence of writes does not flush the page
> cache, resulting in an mmap longer than the new file's length (from
> another process) not properly zero-filling the last page.
>
> Does fuse_finish_open() need additional locking? I am not sure of the
> locking requirements of truncate_pagecache or invalidate_inode_pages2.
>
> The following C++ program reproduces the bug. The FUSE daemon must
> have enabled the ATOMIC_O_TRUNC flag
> https://gist.github.com/chadaustin/8e4ba1e2cd2d023ff6f9eff921eb8bfc
Thanks for the report and the patch.
Here's a simpler patch (we don't actually need the
invalidate_inode_pages2(), because all the pages have already been
truncated).
I tested it with your reproducer and it fixes the issue for me.
Unless you see a problem with it, I'll queue this up for 4.16.
Thanks,
Miklos
---
fs/fuse/dir.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1629,8 +1629,12 @@ int fuse_do_setattr(struct dentry *dentr
return err;
if (attr->ia_valid & ATTR_OPEN) {
- if (fc->atomic_o_trunc)
+ if (fc->atomic_o_trunc) {
+ WARN_ON(!(attr->ia_valid & ATTR_SIZE));
+ WARN_ON(attr->ia_size != 0);
+ truncate_pagecache(inode, 0);
return 0;
+ }
file = NULL;
}
