overlayfs: regression bug from 4bacc9c9 (Make f_path always point to the overlay and f_inode to the underlay)

overlayfs: regression bug from 4bacc9c9 (Make f_path always point to the overlay and f_inode to the underlay)

[Alban Crequy]

Hi,

I'm reporting an issue in overlay fs that was introduced in v4.2 (it
worked on v4.1): when overlay fs is mounted inside a overlay fs, I get
a "no such device or address" error (ENXIO) during open(). After
adding some debug printks, I found that the ENXIO comes from
fs/inode.c:no_open().

The bug was initially reported on:
https://github.com/coreos/rkt/issues/1537

The following commands can reproduce the issue:

# mkdir upper lower work merged
# mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merged/
# cd merged
# mkdir upper2 lower2 work2 merged2
# mount -t overlay overlay
-olowerdir=lower2,upperdir=upper2,workdir=work2 merged2/
# cd merged2
# echo hello > test.txt
bash: test.txt: No such device or address

For debugging purposes, I added a "BUG()" in fs/inode.c:no_open() in
order to see the stack at the moment the ENXIO is returned:

[    0.446166] Call Trace:
[    0.446166]  [<ffffffff8121701f>] do_dentry_open+0x1ff/0x2f0
[    0.446166]  [<ffffffff81232960>] ? inode_init_always+0x1b0/0x1b0
[    0.446166]  [<ffffffff812184c6>] vfs_open+0x56/0x60
[    0.446166]  [<ffffffff812271ee>] path_openat+0x1de/0x1250
[    0.446166]  [<ffffffff8119ee61>] ? filemap_fault+0xb1/0x420
[    0.446166]  [<ffffffff811bcf00>] ? __inc_zone_state+0x20/0x60
[    0.446166]  [<ffffffff81229401>] do_filp_open+0x91/0x100
[    0.446166]  [<ffffffff811fad03>] ? kmem_cache_alloc+0x193/0x210
[    0.446166]  [<ffffffff81228436>] ? getname_flags+0x56/0x1f0
[    0.446166]  [<ffffffff8123634f>] ? __alloc_fd+0x3f/0x100
[    0.446166]  [<ffffffff8121887a>] do_sys_open+0x13a/0x230
[    0.446166]  [<ffffffff8121898e>] SyS_open+0x1e/0x20
[    0.446166]  [<ffffffff81790fee>] entry_SYSCALL_64_fastpath+0x12/0x71

git-bisect found the following:

> 4bacc9c9234c7c8eec44f5ed4e960d9f96fa0f01 is the first bad commit
> commit 4bacc9c9234c7c8eec44f5ed4e960d9f96fa0f01
> Author: David Howells <dhowells@redhat.com>
> Date:   Thu Jun 18 14:32:31 2015 +0100
>
>     overlayfs: Make f_path always point to the overlay
>     and f_inode to the underlay

See patch (2) in https://lwn.net/Articles/648525/:

> (2) The main VFS patch that makes an open file struct referring to a union
>     file have ->f_path point to the union/overlay file whilst ->f_inode and
>     ->f_mapping refer to the subordinate file that does the actual work.

Cheers,
Alban

Re: overlayfs: regression bug from 4bacc9c9 (Make f_path always point to the overlay and f_inode to the underlay)

[Miklos Szeredi]

On Wed, Oct 07, 2015 at 02:23:23PM +0200, Alban Crequy wrote:
> Hi,
> 
> I'm reporting an issue in overlay fs that was introduced in v4.2 (it
> worked on v4.1): when overlay fs is mounted inside a overlay fs, I get
> a "no such device or address" error (ENXIO) during open(). After
> adding some debug printks, I found that the ENXIO comes from
> fs/inode.c:no_open().
> 
> The bug was initially reported on:
> https://github.com/coreos/rkt/issues/1537
> 
> The following commands can reproduce the issue:

Thanks for the excellent report.

See below for a fix.  Please let me know if you see any issues.

Thanks,
Miklos
---

Subject: ovl: fix open in stacked overlay
From: Miklos Szeredi <mszeredi@suse.cz>

If two overlayfs filesystems are stacked on top of each other, then we need
recursion in ovl_d_select_inode().

I guess d_backing_inode() is supposed to do that.  But currently it doesn't
and that functionality is open coded in vfs_open().  This is now copied
into ovl_d_select_inode() to fix this regression.

Reported-by: Alban Crequy <alban.crequy@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay...")
Cc: David Howells <dhowells@redhat.com>
Cc: <stable@vger.kernel.org> # v4.2+
---
 fs/overlayfs/inode.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -363,6 +363,9 @@ struct inode *ovl_d_select_inode(struct
 		ovl_path_upper(dentry, &realpath);
 	}
 
+	if (realpath.dentry->d_flags & DCACHE_OP_SELECT_INODE)
+		return realpath.dentry->d_op->d_select_inode(realpath.dentry, file_flags);
+
 	return d_backing_inode(realpath.dentry);
 }
 

Re: overlayfs: regression bug from 4bacc9c9 (Make f_path always point to the overlay and f_inode to the underlay)

[Alban Crequy]

On 12 October 2015 at 15:50, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Wed, Oct 07, 2015 at 02:23:23PM +0200, Alban Crequy wrote:
>> Hi,
>>
>> I'm reporting an issue in overlay fs that was introduced in v4.2 (it
>> worked on v4.1): when overlay fs is mounted inside a overlay fs, I get
>> a "no such device or address" error (ENXIO) during open(). After
>> adding some debug printks, I found that the ENXIO comes from
>> fs/inode.c:no_open().
>>
>> The bug was initially reported on:
>> https://github.com/coreos/rkt/issues/1537
>>
>> The following commands can reproduce the issue:
>
> Thanks for the excellent report.
>
> See below for a fix.  Please let me know if you see any issues.

Thanks for the fix. I tested it and it fixes the test case with the
commands I wrote in my previous email.

Tested-by: Alban Crequy <alban.crequy@gmail.com>


> Thanks,
> Miklos
> ---
>
> Subject: ovl: fix open in stacked overlay
> From: Miklos Szeredi <mszeredi@suse.cz>
>
> If two overlayfs filesystems are stacked on top of each other, then we need
> recursion in ovl_d_select_inode().
>
> I guess d_backing_inode() is supposed to do that.  But currently it doesn't
> and that functionality is open coded in vfs_open().  This is now copied
> into ovl_d_select_inode() to fix this regression.
>
> Reported-by: Alban Crequy <alban.crequy@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> Fixes: 4bacc9c9234c ("overlayfs: Make f_path always point to the overlay...")
> Cc: David Howells <dhowells@redhat.com>
> Cc: <stable@vger.kernel.org> # v4.2+
> ---
>  fs/overlayfs/inode.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -363,6 +363,9 @@ struct inode *ovl_d_select_inode(struct
>                 ovl_path_upper(dentry, &realpath);
>         }
>
> +       if (realpath.dentry->d_flags & DCACHE_OP_SELECT_INODE)
> +               return realpath.dentry->d_op->d_select_inode(realpath.dentry, file_flags);
> +
>         return d_backing_inode(realpath.dentry);
>  }
>