Re: [POC/RFC PATCH] overlayfs: fix data inconsistency at copy up

[Amir Goldstein]

On Thu, Oct 20, 2016 at 11:46 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Wed, Oct 12, 2016 at 03:33:26PM +0200, Miklos Szeredi wrote:
>> This is a proof of concept patch to fix the following.
>>
>> /ovl is in overlay mount and /ovl/foo exists on the lower layer only.
>>
>>  rofd = open("/ovl/foo", O_RDONLY);
>>  rwfd = open("/ovl/foo", O_WRONLY); /* this causes copy up */
>>  write(rwfd, "bar", 3);
>>  read(rofd, buf, 3);
>>  assert(memcmp(buf, "bar", 3) == 0);
>>
>> Similar problem exists with an MAP_SHARED mmap created from rofd.
>>
>> While this has only caused few problems (yum/dnf failure is the only one I know
>> of) and easily worked around in userspace, many see it as a proof that overlayfs
>> can never be a proper "POSIX" filesystem.
>>
>> To quell those worries, here's a simple patch that should address the above.
>>
>> The only VFS change is that f_op is initialized from f_path.dentry->d_inode
>> instead of file_inode(filp) in open.  The effect of this is that overlayfs can
>> intercept open and other file operations, while the file still effectively
>> belongs to the underlying fs.
>>
>> The patch does not give up on the nice properties of overlayfs, like sharing the
>> page cache with the underlying files.  It does cause copy up in one case where
>> previously there wasn't one and that's the O_RDONLY/MAP_SHARED case.  I haven't
>> done much research into this, but running some tests in chroot didn't trigger
>> this.
>>
>> Comments, testing are welcome.
>
> Hi Miklos,
>
> This looks like a very interesting idea. In fact once file has been copied
> up and writen to, and if I do fstat(rofd), it shows the size of copied up
> file but one can read the contents. So fixing that anomaly would be nice.
>

I think it would be a good idea in general to stabilize the overlay ino/dev
throughout copy-up, same as Miklos suggested to do for directories, to
all files:
pure upper uses upper ino + overlayfs dev
non-pure upper uses lower ino + overlayfs dev