linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* overlayfs: BUG in ovl_whiteout
@ 2010-10-01 18:07 Andy Whitcroft
  2010-10-04  9:23 ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-01 18:07 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, vaurora, neilb, viro

I have been playing with overlayfs for a bit now and there seems to
be something a little dodgy when unmounting and remounting an overlay.
I modified some files in the overlay, immediatly unmounted the overlay and
again immediatly remounted it.  On touching the same files in the overlay
I triggered the following BUG (dmesg fragment at the bottom of this email).
When trying to reproduce this I also triggered a hard hang.

This is the specific BUG in my source:

    static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
    {
    [...]
        /* Just been removed within the same locked region */
        BUG_ON(newdentry->d_inode);
    [...]
    }

All of this is against the V3 patches.

-apw

[ 3828.863867] ------------[ cut here ]------------
[ 3828.863898] kernel BUG at /home/apw/build/maverick/ubuntu-natty/fs/overlayfs/overlayfs.c:1441!
[ 3828.863940] invalid opcode: 0000 [#1] SMP 
[ 3828.863965] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.6/local_cpus
[ 3828.863998] CPU 1 
[ 3828.864010] Modules linked in: overlayfs binfmt_misc ppdev kvm_intel kvm i915 drm_kms_helper snd_hda_codec_idt drm snd_hda_intel i2c_algo_bit snd_hda_codec video snd_hwdep tpm_tis tpm coretemp snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event output snd_seq tpm_bios intel_agp snd_timer lp snd_seq_device parport snd soundcore snd_page_alloc hid_belkin usbhid hid ahci e1000e libahci
[ 3828.864254] 
[ 3828.864266] Pid: 18395, comm: apt-get Not tainted 2.6.36-0-generic #1~overlayfs201010011723 BB Name To be filled by O.E.M./Product Name To Be Filled By O.E.M.
[ 3828.864330] RIP: 0010:[<ffffffffa02ca173>]  [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
[ 3828.864379] RSP: 0018:ffff8800670e1c28  EFLAGS: 00010282
[ 3828.864405] RAX: ffff8800263f3600 RBX: ffff88006711ec00 RCX: 0000000000000000
[ 3828.864439] RDX: ffff8800263f3600 RSI: ffff88006aefe4c2 RDI: ffff8800263f3608
[ 3828.864472] RBP: ffff8800670e1c58 R08: 0000000000000650 R09: ffff8800765d7e80
[ 3828.864506] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800263f3600
[ 3828.864539] R13: ffff88006e097240 R14: ffff880063881780 R15: 00000000fffffff4
[ 3828.864573] FS:  0000000000000000(0000) GS:ffff880001e80000(0063) knlGS:00000000f73d26d0
[ 3828.864612] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 3828.864639] CR2: 00000000f770e4c0 CR3: 000000007808d000 CR4: 00000000000406e0
[ 3828.864673] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 3828.864706] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 3828.864740] Process apt-get (pid: 18395, threadinfo ffff8800670e0000, task ffff88006f4f2dc0)
[ 3828.864779] Stack:
[ 3828.864790]  ffff8800670e1c58 ffff8800263bba80 ffff8800263bba80 0000000000000000
[ 3828.864832] <0> ffff8800263f3600 ffff880063881780 ffff8800670e1cf8 ffffffffa02cbbe9
[ 3828.864877] <0> ffff8800670e1d08 0000000000000001 ffff880063881701 ffff8800263f3600
[ 3828.864924] Call Trace:
[ 3828.864940]  [<ffffffffa02cbbe9>] ovl_rename+0x319/0x3d0 [overlayfs]
[ 3828.864976]  [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
[ 3828.865009]  [<ffffffff81163db9>] ? dentry_permission+0x99/0xc0
[ 3828.865038]  [<ffffffff81163a4b>] vfs_rename_other+0xcb/0x130
[ 3828.865067]  [<ffffffff81164fdb>] vfs_rename+0x14b/0x240
[ 3828.865094]  [<ffffffff81163555>] ? __lookup_hash+0x55/0xe0
[ 3828.865121]  [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
[ 3828.865154]  [<ffffffff81166d04>] sys_renameat+0x254/0x280
[ 3828.865184]  [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
[ 3828.865212]  [<ffffffff812c3bcd>] ? _atomic_dec_and_lock+0x4d/0x80
[ 3828.865243]  [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
[ 3828.865271]  [<ffffffff81157993>] ? sys_fchmodat+0x73/0x110
[ 3828.865300]  [<ffffffff81048173>] ? sys32_stat64+0x33/0x40
[ 3828.865328]  [<ffffffff81166d4b>] sys_rename+0x1b/0x20
[ 3828.865354]  [<ffffffff81047230>] sysenter_dispatch+0x7/0x2e
[ 3828.865382] Code: eb d4 eb 04 90 90 90 90 48 8b 35 a1 2a 00 00 45 31 c0 b9 01 00 00 00 48 c7 c2 25 ce 2c a0 4c 89 e7 e8 92 fa ea e0 41 89 c7 eb 91 <0f> 0b eb fe 41 89 c7 eb 90 eb 02 90 90 55 48 89 e5 48 83 ec 20 
[ 3828.865610] RIP  [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
[ 3828.866873]  RSP <ffff8800670e1c28>
[ 3828.889817] ---[ end trace ae40f79401a99f83 ]---

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: overlayfs: BUG in ovl_whiteout
  2010-10-01 18:07 overlayfs: BUG in ovl_whiteout Andy Whitcroft
@ 2010-10-04  9:23 ` Miklos Szeredi
  2010-10-04 17:33   ` Andy Whitcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-04  9:23 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel

On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> I have been playing with overlayfs for a bit now and there seems to
> be something a little dodgy when unmounting and remounting an overlay.
> I modified some files in the overlay, immediatly unmounted the overlay and
> again immediatly remounted it.  On touching the same files in the overlay
> I triggered the following BUG (dmesg fragment at the bottom of this email).
> When trying to reproduce this I also triggered a hard hang.

Thanks for the bug report, Andy.

Please change that BUG_ON to a WARN_ON.  This will make this a
harmless error message in the log instead of a hang.

> 
> This is the specific BUG in my source:
> 
>     static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
>     {
>     [...]
>         /* Just been removed within the same locked region */
>         BUG_ON(newdentry->d_inode);
>     [...]
>     }
> 
> All of this is against the V3 patches.

What are the filesystems used as the upper and lower layers of the
overlay?

Thanks,
Miklos

> 
> -apw
> 
> [ 3828.863867] ------------[ cut here ]------------
> [ 3828.863898] kernel BUG at /home/apw/build/maverick/ubuntu-natty/fs/overlayfs/overlayfs.c:1441!
> [ 3828.863940] invalid opcode: 0000 [#1] SMP 
> [ 3828.863965] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.6/local_cpus
> [ 3828.863998] CPU 1 
> [ 3828.864010] Modules linked in: overlayfs binfmt_misc ppdev kvm_intel kvm i915 drm_kms_helper snd_hda_codec_idt drm snd_hda_intel i2c_algo_bit snd_hda_codec video snd_hwdep tpm_tis tpm coretemp snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event output snd_seq tpm_bios intel_agp snd_timer lp snd_seq_device parport snd soundcore snd_page_alloc hid_belkin usbhid hid ahci e1000e libahci
> [ 3828.864254] 
> [ 3828.864266] Pid: 18395, comm: apt-get Not tainted 2.6.36-0-generic #1~overlayfs201010011723 BB Name To be filled by O.E.M./Product Name To Be Filled By O.E.M.
> [ 3828.864330] RIP: 0010:[<ffffffffa02ca173>]  [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
> [ 3828.864379] RSP: 0018:ffff8800670e1c28  EFLAGS: 00010282
> [ 3828.864405] RAX: ffff8800263f3600 RBX: ffff88006711ec00 RCX: 0000000000000000
> [ 3828.864439] RDX: ffff8800263f3600 RSI: ffff88006aefe4c2 RDI: ffff8800263f3608
> [ 3828.864472] RBP: ffff8800670e1c58 R08: 0000000000000650 R09: ffff8800765d7e80
> [ 3828.864506] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800263f3600
> [ 3828.864539] R13: ffff88006e097240 R14: ffff880063881780 R15: 00000000fffffff4
> [ 3828.864573] FS:  0000000000000000(0000) GS:ffff880001e80000(0063) knlGS:00000000f73d26d0
> [ 3828.864612] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
> [ 3828.864639] CR2: 00000000f770e4c0 CR3: 000000007808d000 CR4: 00000000000406e0
> [ 3828.864673] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 3828.864706] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 3828.864740] Process apt-get (pid: 18395, threadinfo ffff8800670e0000, task ffff88006f4f2dc0)
> [ 3828.864779] Stack:
> [ 3828.864790]  ffff8800670e1c58 ffff8800263bba80 ffff8800263bba80 0000000000000000
> [ 3828.864832] <0> ffff8800263f3600 ffff880063881780 ffff8800670e1cf8 ffffffffa02cbbe9
> [ 3828.864877] <0> ffff8800670e1d08 0000000000000001 ffff880063881701 ffff8800263f3600
> [ 3828.864924] Call Trace:
> [ 3828.864940]  [<ffffffffa02cbbe9>] ovl_rename+0x319/0x3d0 [overlayfs]
> [ 3828.864976]  [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
> [ 3828.865009]  [<ffffffff81163db9>] ? dentry_permission+0x99/0xc0
> [ 3828.865038]  [<ffffffff81163a4b>] vfs_rename_other+0xcb/0x130
> [ 3828.865067]  [<ffffffff81164fdb>] vfs_rename+0x14b/0x240
> [ 3828.865094]  [<ffffffff81163555>] ? __lookup_hash+0x55/0xe0
> [ 3828.865121]  [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
> [ 3828.865154]  [<ffffffff81166d04>] sys_renameat+0x254/0x280
> [ 3828.865184]  [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
> [ 3828.865212]  [<ffffffff812c3bcd>] ? _atomic_dec_and_lock+0x4d/0x80
> [ 3828.865243]  [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
> [ 3828.865271]  [<ffffffff81157993>] ? sys_fchmodat+0x73/0x110
> [ 3828.865300]  [<ffffffff81048173>] ? sys32_stat64+0x33/0x40
> [ 3828.865328]  [<ffffffff81166d4b>] sys_rename+0x1b/0x20
> [ 3828.865354]  [<ffffffff81047230>] sysenter_dispatch+0x7/0x2e
> [ 3828.865382] Code: eb d4 eb 04 90 90 90 90 48 8b 35 a1 2a 00 00 45 31 c0 b9 01 00 00 00 48 c7 c2 25 ce 2c a0 4c 89 e7 e8 92 fa ea e0 41 89 c7 eb 91 <0f> 0b eb fe 41 89 c7 eb 90 eb 02 90 90 55 48 89 e5 48 83 ec 20 
> [ 3828.865610] RIP  [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
> [ 3828.866873]  RSP <ffff8800670e1c28>
> [ 3828.889817] ---[ end trace ae40f79401a99f83 ]---
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: overlayfs: BUG in ovl_whiteout
  2010-10-04  9:23 ` Miklos Szeredi
@ 2010-10-04 17:33   ` Andy Whitcroft
  2010-10-04 20:16     ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-04 17:33 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Mon, Oct 04, 2010 at 11:23:16AM +0200, Miklos Szeredi wrote:
> On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> > I have been playing with overlayfs for a bit now and there seems to
> > be something a little dodgy when unmounting and remounting an overlay.
> > I modified some files in the overlay, immediatly unmounted the overlay and
> > again immediatly remounted it.  On touching the same files in the overlay
> > I triggered the following BUG (dmesg fragment at the bottom of this email).
> > When trying to reproduce this I also triggered a hard hang.
> 
> Thanks for the bug report, Andy.
> 
> Please change that BUG_ON to a WARN_ON.  This will make this a
> harmless error message in the log instead of a hang.

Applied your patch to move these to WARN_ON.  Now I get a WARN_ON after
the remount.  Now that we don't explode I get the following error from
the triggering command.  I assume that this is the rename which has
triggered the issue:

W: Failed to fetch http://192.168.0.60:3142/archive.ubuntu.com/ubuntu/dists/maverick/Release rename failed, File exists (/var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release -> /var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release).

I note that the file does indeed exist as a real file in both layers.  I
have not as yet managed to generate a naive test that also triggers
this.

-apw

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: overlayfs: BUG in ovl_whiteout
  2010-10-04 17:33   ` Andy Whitcroft
@ 2010-10-04 20:16     ` Miklos Szeredi
  2010-10-06 12:29       ` Andy Whitcroft
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-04 20:16 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel

On Mon, 4 Oct 2010, Andy Whitcroft wrote:
> On Mon, Oct 04, 2010 at 11:23:16AM +0200, Miklos Szeredi wrote:
> > On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> > > I have been playing with overlayfs for a bit now and there seems to
> > > be something a little dodgy when unmounting and remounting an overlay.
> > > I modified some files in the overlay, immediatly unmounted the overlay and
> > > again immediatly remounted it.  On touching the same files in the overlay
> > > I triggered the following BUG (dmesg fragment at the bottom of this email).
> > > When trying to reproduce this I also triggered a hard hang.
> > 
> > Thanks for the bug report, Andy.
> > 
> > Please change that BUG_ON to a WARN_ON.  This will make this a
> > harmless error message in the log instead of a hang.
> 
> Applied your patch to move these to WARN_ON.  Now I get a WARN_ON after
> the remount.  Now that we don't explode I get the following error from
> the triggering command.  I assume that this is the rename which has
> triggered the issue:
> 
> W: Failed to fetch http://192.168.0.60:3142/archive.ubuntu.com/ubuntu/dists/maverick/Release rename failed, File exists (/var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release -> /var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release).
> 
> I note that the file does indeed exist as a real file in both layers.  I
> have not as yet managed to generate a naive test that also triggers
> this.

OK, my guess is that it's a 'rename to self' which was not properly
implemented.

Does the following patch make a difference?

Thanks,
Miklos

---
 fs/namei.c               |   19 ++++++++++++++-----
 fs/overlayfs/overlayfs.c |   19 ++++++++++++++++++-
 include/linux/fs.h       |    3 ++-
 3 files changed, 34 insertions(+), 7 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/fs/namei.c	2010-10-04 21:46:13.000000000 +0200
@@ -2620,6 +2620,17 @@ static int vfs_rename_other(struct inode
 	return error;
 }
 
+bool vfs_is_same_inode(struct dentry *d1, struct dentry *d2)
+{
+	BUG_ON(d1->d_sb != d2->d_sb);
+
+	if (d1->d_sb->s_op->is_same_inode)
+		return d1->d_sb->s_op->is_same_inode(d1, d2);
+	else
+		return d1->d_inode == d2->d_inode;
+}
+EXPORT_SYMBOL(vfs_is_same_inode);
+
 int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	       struct inode *new_dir, struct dentry *new_dentry)
 {
@@ -2627,11 +2638,9 @@ int vfs_rename(struct inode *old_dir, st
 	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
 	const unsigned char *old_name;
 
-	if (old_dentry->d_inode == new_dentry->d_inode &&
-	    !(old_dir->i_sb->s_type->fs_flags & FS_RENAME_SELF_ALLOW)) {
- 		return 0;
-	}
- 
+	if (vfs_is_same_inode(old_dentry, new_dentry))
+		return 0;
+
 	error = may_delete(old_dir, old_dentry, is_dir);
 	if (error)
 		return error;
Index: linux-2.6/fs/overlayfs/overlayfs.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/overlayfs.c	2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/fs/overlayfs/overlayfs.c	2010-10-04 21:00:05.000000000 +0200
@@ -1962,10 +1962,28 @@ static int ovl_statfs(struct dentry *den
 	return path.dentry->d_sb->s_op->statfs(path.dentry, buf);
 }
 
+static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
+{
+	struct dentry *upperd1;
+	struct dentry *upperd2;
+
+	upperd1 = ovl_dentry_upper(d1);
+	upperd2 = ovl_dentry_upper(d2);
+
+	if (upperd1 && upperd2)
+		return vfs_is_same_inode(upperd1, upperd2);
+
+	if (upperd1 || upperd2)
+		return false;
+
+	return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
+}
+
 static const struct super_operations ovl_super_operations = {
 	.put_super	= ovl_put_super,
 	.remount_fs	= ovl_remount_fs,
 	.statfs		= ovl_statfs,
+	.is_same_inode	= ovl_is_same_inode,
 };
 
 struct ovl_config {
@@ -2155,7 +2173,6 @@ static int ovl_get_sb(struct file_system
 static struct file_system_type ovl_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "overlayfs",
-	.fs_flags	= FS_RENAME_SELF_ALLOW,
 	.get_sb		= ovl_get_sb,
 	.kill_sb	= kill_anon_super,
 };
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2010-10-04 21:47:40.000000000 +0200
@@ -179,7 +179,6 @@ struct inodes_stat_t {
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move()
 					 * during rename() internally.
 					 */
-#define FS_RENAME_SELF_ALLOW	65536	/* Allow rename to same inode */
 
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
@@ -1579,6 +1578,7 @@ struct super_operations {
 	ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
 #endif
 	int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
+	bool (*is_same_inode)(struct dentry *, struct dentry *);
 };
 
 /*
@@ -1816,6 +1816,7 @@ extern int vfs_statfs(struct path *, str
 extern int statfs_by_dentry(struct dentry *, struct kstatfs *);
 extern int freeze_super(struct super_block *super);
 extern int thaw_super(struct super_block *super);
+extern bool vfs_is_same_inode(struct dentry *d1, struct dentry *d2);
 
 extern int current_umask(void);
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: overlayfs: BUG in ovl_whiteout
  2010-10-04 20:16     ` Miklos Szeredi
@ 2010-10-06 12:29       ` Andy Whitcroft
  2010-10-06 16:51         ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-06 12:29 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel

On Mon, Oct 04, 2010 at 10:16:29PM +0200, Miklos Szeredi wrote:
> 
> OK, my guess is that it's a 'rename to self' which was not properly
> implemented.
> 
> Does the following patch make a difference?

That one panic'd immediatly.  Seems that we can have missmatched lowers
as well.  I applied the below over the top, and that seems to pass
testing without tripping the WARN_ON.

-apw

commit 03375858c62f617846b5dc4968fe8178eda51a7c
Author: Andy Whitcroft <apw@canonical.com>
Date:   Tue Oct 5 14:54:08 2010 +0100

    overlayfs: handle missing lower inodes in ovl_is_same_inode
    
    Signed-off-by: Andy Whitcroft <apw@canonical.com>

diff --git a/fs/overlayfs/overlayfs.c b/fs/overlayfs/overlayfs.c
index c10cc7b..f596222 100644
--- a/fs/overlayfs/overlayfs.c
+++ b/fs/overlayfs/overlayfs.c
@@ -1964,19 +1964,25 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
 
 static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
 {
-	struct dentry *upperd1;
-	struct dentry *upperd2;
+	struct dentry *od1;
+	struct dentry *od2;
 
-	upperd1 = ovl_dentry_upper(d1);
-	upperd2 = ovl_dentry_upper(d2);
+	od1 = ovl_dentry_upper(d1);
+	od2 = ovl_dentry_upper(d2);
 
-	if (upperd1 && upperd2)
-		return vfs_is_same_inode(upperd1, upperd2);
+	if (od1 && od2)
+		return vfs_is_same_inode(od1, od2);
 
-	if (upperd1 || upperd2)
+	if (od1 || od2)
 		return false;
 
-	return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
+	od1 = ovl_dentry_lower(d1);
+	od2 = ovl_dentry_lower(d2);
+
+	if (od1 && od2)
+		return vfs_is_same_inode(od1, od2);
+
+	return false;
 }
 
 static const struct super_operations ovl_super_operations = {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: overlayfs: BUG in ovl_whiteout
  2010-10-06 12:29       ` Andy Whitcroft
@ 2010-10-06 16:51         ` Miklos Szeredi
  0 siblings, 0 replies; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-06 16:51 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel

On Wed, 6 Oct 2010, Andy Whitcroft wrote:
> On Mon, Oct 04, 2010 at 10:16:29PM +0200, Miklos Szeredi wrote:
> > 
> > OK, my guess is that it's a 'rename to self' which was not properly
> > implemented.
> > 
> > Does the following patch make a difference?
> 
> That one panic'd immediatly.  Seems that we can have missmatched lowers
> as well.  I applied the below over the top, and that seems to pass
> testing without tripping the WARN_ON.
> 
> -apw
> 
> commit 03375858c62f617846b5dc4968fe8178eda51a7c
> Author: Andy Whitcroft <apw@canonical.com>
> Date:   Tue Oct 5 14:54:08 2010 +0100
> 
>     overlayfs: handle missing lower inodes in ovl_is_same_inode
>     
>     Signed-off-by: Andy Whitcroft <apw@canonical.com>

OK, that's because d1 or d2 is negative, otherwise either the upper or
the lower dentry _must_ exist.  In the version I committed to the
overlayfs.v3 branch vfs_is_same_inode() checks whether the dentries
are positive and only calls ->is_same_inode() if they are.

Can you please try pull from that branch and check if it's still buggy
without your patch?

Thanks,
Miklos



> 
> diff --git a/fs/overlayfs/overlayfs.c b/fs/overlayfs/overlayfs.c
> index c10cc7b..f596222 100644
> --- a/fs/overlayfs/overlayfs.c
> +++ b/fs/overlayfs/overlayfs.c
> @@ -1964,19 +1964,25 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
>  
>  static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
>  {
> -	struct dentry *upperd1;
> -	struct dentry *upperd2;
> +	struct dentry *od1;
> +	struct dentry *od2;
>  
> -	upperd1 = ovl_dentry_upper(d1);
> -	upperd2 = ovl_dentry_upper(d2);
> +	od1 = ovl_dentry_upper(d1);
> +	od2 = ovl_dentry_upper(d2);
>  
> -	if (upperd1 && upperd2)
> -		return vfs_is_same_inode(upperd1, upperd2);
> +	if (od1 && od2)
> +		return vfs_is_same_inode(od1, od2);
>  
> -	if (upperd1 || upperd2)
> +	if (od1 || od2)
>  		return false;
>  
> -	return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
> +	od1 = ovl_dentry_lower(d1);
> +	od2 = ovl_dentry_lower(d2);
> +
> +	if (od1 && od2)
> +		return vfs_is_same_inode(od1, od2);
> +
> +	return false;
>  }
>  
>  static const struct super_operations ovl_super_operations = {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-10-06 16:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-01 18:07 overlayfs: BUG in ovl_whiteout Andy Whitcroft
2010-10-04  9:23 ` Miklos Szeredi
2010-10-04 17:33   ` Andy Whitcroft
2010-10-04 20:16     ` Miklos Szeredi
2010-10-06 12:29       ` Andy Whitcroft
2010-10-06 16:51         ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).