Patchwork ext4: fix a bug when we try to open a file with O_TMPFILE flag

login
register
mail settings
Submitter Zheng Liu
Date July 19, 2013, 12:14 p.m.
Message ID <20130719121405.GA3856@gmail.com>
Download mbox | patch
Permalink /patch/260250/
State Accepted
Headers show

Comments

Zheng Liu - July 19, 2013, 12:14 p.m.
Hi Dave,

After applied this patch, the problem has been fixed in my own sand box.
But that would be great if you could give it a try.  I want to make sure
that this patch can fix the problem.  It looks like there has the same
problem in ext3.  So if this patch is fine, I will generate a patch for
ext3 file system.

Thanks,
						- Zheng

From: Zheng Liu <wenqing.lz@taobao.com>

When we try to open a file with O_TMPFILE flag, we will trigger a bug.
The root cause is that in ext4_orphan_add() we check ->i_nlink == 0 and
this check always fails because we set ->i_nlink = 1 in
inode_init_always().  We can use the following program to trigger it:

int main(int argc, char *argv[])
{
	int fd;

	fd = open(argv[1], O_TMPFILE, 0666);
	if (fd < 0) {
		perror("open ");
		return -1;
	}
	close(fd);
	return 0;
}

The oops message looks like this:

kernel BUG at fs/ext4/namei.c:2572!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in: dlci bridge stp hidp cmtp kernelcapi l2tp_ppp l2tp_netlink l2tp_core sctp libcrc32c rfcomm tun fuse nfnetli
nk can_raw ipt_ULOG can_bcm x25 scsi_transport_iscsi ipx p8023 p8022 appletalk phonet psnap vmw_vsock_vmci_transport af_key vmw_vmci rose vsock atm can netrom ax25 af_rxrpc ir
da pppoe pppox ppp_generic slhc bluetooth nfc rfkill rds caif_socket caif crc_ccitt af_802154 llc2 llc snd_hda_codec_realtek snd_hda_intel snd_hda_codec serio_raw snd_pcm pcsp
kr edac_core snd_page_alloc snd_timer snd soundcore r8169 mii sr_mod cdrom pata_atiixp radeon backlight drm_kms_helper ttm
CPU: 1 PID: 1812571 Comm: trinity-child2 Not tainted 3.11.0-rc1+ #12
Hardware name: Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H, BIOS F12a 04/23/2010
task: ffff88007dfe69a0 ti: ffff88010f7b6000 task.ti: ffff88010f7b6000
RIP: 0010:[<ffffffff8125ce69>]  [<ffffffff8125ce69>] ext4_orphan_add+0x299/0x2b0
RSP: 0018:ffff88010f7b7cf8  EFLAGS: 00010202
RAX: 0000000000000000 RBX: ffff8800966d3020 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff88007dfe70b8 RDI: 0000000000000001
RBP: ffff88010f7b7d40 R08: ffff880126a3c4e0 R09: ffff88010f7b7ca0
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801271fd668
R13: ffff8800966d2f78 R14: ffff88011d7089f0 R15: ffff88007dfe69a0
FS:  00007f70441a3740(0000) GS:ffff88012a800000(0000) knlGS:00000000f77c96c0
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000002834000 CR3: 0000000107964000 CR4: 00000000000007e0
DR0: 0000000000780000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
Stack:
 0000000000002000 00000020810b6dde 0000000000000000 ffff88011d46db00
 ffff8800966d3020 ffff88011d7089f0 ffff88009c7f4c10 ffff88010f7b7f2c
 ffff88007dfe69a0 ffff88010f7b7da8 ffffffff8125cfac ffff880100000004
Call Trace:
 [<ffffffff8125cfac>] ext4_tmpfile+0x12c/0x180
 [<ffffffff811cba78>] path_openat+0x238/0x700
 [<ffffffff8100afc4>] ? native_sched_clock+0x24/0x80
 [<ffffffff811cc647>] do_filp_open+0x47/0xa0
 [<ffffffff811db73f>] ? __alloc_fd+0xaf/0x200
 [<ffffffff811ba2e4>] do_sys_open+0x124/0x210
 [<ffffffff81010725>] ? syscall_trace_enter+0x25/0x290
 [<ffffffff811ba3ee>] SyS_open+0x1e/0x20
 [<ffffffff816ca8d4>] tracesys+0xdd/0xe2
 [<ffffffff81001001>] ? start_thread_common.constprop.6+0x1/0xa0
Code: 04 00 00 00 89 04 24 31 c0 e8 c4 77 04 00 e9 43 fe ff ff 66 25 00 d0 66 3d 00 80 0f 84 0e fe ff ff 83 7b 48 00 0f 84 04 fe ff ff <0f> 0b 49 8b 8c 24 50 07 00 00 e9 88 fe ff ff 0f 1f 84 00 00 00

Here we couldn't call clear_nlink() directly because in d_tmpfile() we
will call inode_dec_link_count() to decrease ->i_nlink.  So this commit
tries to call d_tmpfile() before ext4_orphan_add() to fix this problem.

Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ext4/namei.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Darrick J. Wong - July 19, 2013, 7:31 p.m.
On Fri, Jul 19, 2013 at 08:14:05PM +0800, Zheng Liu wrote:
> Hi Dave,
> 
> After applied this patch, the problem has been fixed in my own sand box.
> But that would be great if you could give it a try.  I want to make sure
> that this patch can fix the problem.  It looks like there has the same
> problem in ext3.  So if this patch is fine, I will generate a patch for
> ext3 file system.

Seems to fix the bug for me, both with your test case and also with trinity
runs, so...

Tested-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
> Thanks,
> 						- Zheng
> 
> From: Zheng Liu <wenqing.lz@taobao.com>
> 
> When we try to open a file with O_TMPFILE flag, we will trigger a bug.
> The root cause is that in ext4_orphan_add() we check ->i_nlink == 0 and
> this check always fails because we set ->i_nlink = 1 in
> inode_init_always().  We can use the following program to trigger it:
> 
> int main(int argc, char *argv[])
> {
> 	int fd;
> 
> 	fd = open(argv[1], O_TMPFILE, 0666);
> 	if (fd < 0) {
> 		perror("open ");
> 		return -1;
> 	}
> 	close(fd);
> 	return 0;
> }
> 
> The oops message looks like this:
> 
> kernel BUG at fs/ext4/namei.c:2572!
> invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> Modules linked in: dlci bridge stp hidp cmtp kernelcapi l2tp_ppp l2tp_netlink l2tp_core sctp libcrc32c rfcomm tun fuse nfnetli
> nk can_raw ipt_ULOG can_bcm x25 scsi_transport_iscsi ipx p8023 p8022 appletalk phonet psnap vmw_vsock_vmci_transport af_key vmw_vmci rose vsock atm can netrom ax25 af_rxrpc ir
> da pppoe pppox ppp_generic slhc bluetooth nfc rfkill rds caif_socket caif crc_ccitt af_802154 llc2 llc snd_hda_codec_realtek snd_hda_intel snd_hda_codec serio_raw snd_pcm pcsp
> kr edac_core snd_page_alloc snd_timer snd soundcore r8169 mii sr_mod cdrom pata_atiixp radeon backlight drm_kms_helper ttm
> CPU: 1 PID: 1812571 Comm: trinity-child2 Not tainted 3.11.0-rc1+ #12
> Hardware name: Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H, BIOS F12a 04/23/2010
> task: ffff88007dfe69a0 ti: ffff88010f7b6000 task.ti: ffff88010f7b6000
> RIP: 0010:[<ffffffff8125ce69>]  [<ffffffff8125ce69>] ext4_orphan_add+0x299/0x2b0
> RSP: 0018:ffff88010f7b7cf8  EFLAGS: 00010202
> RAX: 0000000000000000 RBX: ffff8800966d3020 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffff88007dfe70b8 RDI: 0000000000000001
> RBP: ffff88010f7b7d40 R08: ffff880126a3c4e0 R09: ffff88010f7b7ca0
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801271fd668
> R13: ffff8800966d2f78 R14: ffff88011d7089f0 R15: ffff88007dfe69a0
> FS:  00007f70441a3740(0000) GS:ffff88012a800000(0000) knlGS:00000000f77c96c0
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000002834000 CR3: 0000000107964000 CR4: 00000000000007e0
> DR0: 0000000000780000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> Stack:
>  0000000000002000 00000020810b6dde 0000000000000000 ffff88011d46db00
>  ffff8800966d3020 ffff88011d7089f0 ffff88009c7f4c10 ffff88010f7b7f2c
>  ffff88007dfe69a0 ffff88010f7b7da8 ffffffff8125cfac ffff880100000004
> Call Trace:
>  [<ffffffff8125cfac>] ext4_tmpfile+0x12c/0x180
>  [<ffffffff811cba78>] path_openat+0x238/0x700
>  [<ffffffff8100afc4>] ? native_sched_clock+0x24/0x80
>  [<ffffffff811cc647>] do_filp_open+0x47/0xa0
>  [<ffffffff811db73f>] ? __alloc_fd+0xaf/0x200
>  [<ffffffff811ba2e4>] do_sys_open+0x124/0x210
>  [<ffffffff81010725>] ? syscall_trace_enter+0x25/0x290
>  [<ffffffff811ba3ee>] SyS_open+0x1e/0x20
>  [<ffffffff816ca8d4>] tracesys+0xdd/0xe2
>  [<ffffffff81001001>] ? start_thread_common.constprop.6+0x1/0xa0
> Code: 04 00 00 00 89 04 24 31 c0 e8 c4 77 04 00 e9 43 fe ff ff 66 25 00 d0 66 3d 00 80 0f 84 0e fe ff ff 83 7b 48 00 0f 84 04 fe ff ff <0f> 0b 49 8b 8c 24 50 07 00 00 e9 88 fe ff ff 0f 1f 84 00 00 00
> 
> Here we couldn't call clear_nlink() directly because in d_tmpfile() we
> will call inode_dec_link_count() to decrease ->i_nlink.  So this commit
> tries to call d_tmpfile() before ext4_orphan_add() to fix this problem.
> 
> Reported-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/ext4/namei.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 234b834..35f55a0 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2316,11 +2316,11 @@ retry:
>  		inode->i_op = &ext4_file_inode_operations;
>  		inode->i_fop = &ext4_file_operations;
>  		ext4_set_aops(inode);
> +		d_tmpfile(dentry, inode);
>  		err = ext4_orphan_add(handle, inode);
>  		if (err)
>  			goto err_drop_inode;
>  		mark_inode_dirty(inode);
> -		d_tmpfile(dentry, inode);
>  		unlock_new_inode(inode);
>  	}
>  	if (handle)
> -- 
> 1.7.9.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Jones - July 19, 2013, 8:01 p.m.
On Fri, Jul 19, 2013 at 12:31:06PM -0700, Darrick J. Wong wrote:
 > On Fri, Jul 19, 2013 at 08:14:05PM +0800, Zheng Liu wrote:
 > > Hi Dave,
 > > 
 > > After applied this patch, the problem has been fixed in my own sand box.
 > > But that would be great if you could give it a try.  I want to make sure
 > > that this patch can fix the problem.  It looks like there has the same
 > > problem in ext3.  So if this patch is fine, I will generate a patch for
 > > ext3 file system.
 > 
 > Seems to fix the bug for me, both with your test case and also with trinity
 > runs, so...

Yeah, seems to be working here too. 

 > Tested-by: Darrick J. Wong <darrick.wong@oracle.com>
 > Tested-by: Darrick J. Wong <darrick.wong@oracle.com>

likewise,

Tested-by: Dave Jones <davej@redhat.com>

	Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Al Viro - July 19, 2013, 11:36 p.m.
On Fri, Jul 19, 2013 at 08:14:05PM +0800, Zheng Liu wrote:
> Hi Dave,
> 
> After applied this patch, the problem has been fixed in my own sand box.
> But that would be great if you could give it a try.  I want to make sure
> that this patch can fix the problem.  It looks like there has the same
> problem in ext3.  So if this patch is fine, I will generate a patch for
> ext3 file system.

Nice catch, and you have my ACK.  Which tree do you prefer that to go
through?  vfs and ext4 are the obvious candidates...
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu - July 20, 2013, 4:37 p.m.
Hi Al,

On Sat, Jul 20, 2013 at 12:36:13AM +0100, Al Viro wrote:
> On Fri, Jul 19, 2013 at 08:14:05PM +0800, Zheng Liu wrote:
> > Hi Dave,
> > 
> > After applied this patch, the problem has been fixed in my own sand box.
> > But that would be great if you could give it a try.  I want to make sure
> > that this patch can fix the problem.  It looks like there has the same
> > problem in ext3.  So if this patch is fine, I will generate a patch for
> > ext3 file system.
> 
> Nice catch, and you have my ACK.  Which tree do you prefer that to go
> through?  vfs and ext4 are the obvious candidates...

Now ext4 tree has not yet rebased against 3.11-rcX.  So it seems that
vfs tree is better if we want to fix this bug in mainline kernel as soon
as possible.  This patch is against mainline kernel.  I think it should
be applied into vfs tree cleanly.  Please let me know if I need to rebase
it against vfs tree.  BTW, the patch for ext3 file system will be sent
out soon.  Please pick it up.

Thanks,
                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - July 21, 2013, 12:56 a.m.
On Sun, Jul 21, 2013 at 12:37:37AM +0800, Zheng Liu wrote:
> 
> Now ext4 tree has not yet rebased against 3.11-rcX.  So it seems that
> vfs tree is better if we want to fix this bug in mainline kernel as soon
> as possible.

I was going to rebase the ext4 tree to Linus's current tree just to
get this patch in.  I think it would be good to get this fix before
-rc2, so either you or I should get this to Linus ASAP.

So Al, if you're going to send vfs patches to Linus this weekend (say,
to fix the equivalent bug for ext3), go for it.  Otherwise, let me
know and I'll send Linus a one-commit pull request just to get this
fix in.

Cheers,

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jon ernst - July 23, 2013, 7:51 a.m.
Hi Zheng,

I updated my kernel code to this commit, built kernel and loaded this kernel.
I checked "uname -r", it is 3.11.0-rc1+ But when I run your test code,
I always got "O_TMPFILE" undeclared.
Could you please help, what could be wrong? (I included fcntl.h)

Thank you!


On Fri, Jul 19, 2013 at 12:14 PM, Zheng Liu <gnehzuil.liu@gmail.com> wrote:
> Hi Dave,
>
> After applied this patch, the problem has been fixed in my own sand box.
> But that would be great if you could give it a try.  I want to make sure
> that this patch can fix the problem.  It looks like there has the same
> problem in ext3.  So if this patch is fine, I will generate a patch for
> ext3 file system.
>
> Thanks,
>                                                 - Zheng
>
> From: Zheng Liu <wenqing.lz@taobao.com>
>
> When we try to open a file with O_TMPFILE flag, we will trigger a bug.
> The root cause is that in ext4_orphan_add() we check ->i_nlink == 0 and
> this check always fails because we set ->i_nlink = 1 in
> inode_init_always().  We can use the following program to trigger it:
>
> int main(int argc, char *argv[])
> {
>         int fd;
>
>         fd = open(argv[1], O_TMPFILE, 0666);
>         if (fd < 0) {
>                 perror("open ");
>                 return -1;
>         }
>         close(fd);
>         return 0;
> }
>
> The oops message looks like this:
>
> kernel BUG at fs/ext4/namei.c:2572!
> invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> Modules linked in: dlci bridge stp hidp cmtp kernelcapi l2tp_ppp l2tp_netlink l2tp_core sctp libcrc32c rfcomm tun fuse nfnetli
> nk can_raw ipt_ULOG can_bcm x25 scsi_transport_iscsi ipx p8023 p8022 appletalk phonet psnap vmw_vsock_vmci_transport af_key vmw_vmci rose vsock atm can netrom ax25 af_rxrpc ir
> da pppoe pppox ppp_generic slhc bluetooth nfc rfkill rds caif_socket caif crc_ccitt af_802154 llc2 llc snd_hda_codec_realtek snd_hda_intel snd_hda_codec serio_raw snd_pcm pcsp
> kr edac_core snd_page_alloc snd_timer snd soundcore r8169 mii sr_mod cdrom pata_atiixp radeon backlight drm_kms_helper ttm
> CPU: 1 PID: 1812571 Comm: trinity-child2 Not tainted 3.11.0-rc1+ #12
> Hardware name: Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H, BIOS F12a 04/23/2010
> task: ffff88007dfe69a0 ti: ffff88010f7b6000 task.ti: ffff88010f7b6000
> RIP: 0010:[<ffffffff8125ce69>]  [<ffffffff8125ce69>] ext4_orphan_add+0x299/0x2b0
> RSP: 0018:ffff88010f7b7cf8  EFLAGS: 00010202
> RAX: 0000000000000000 RBX: ffff8800966d3020 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffff88007dfe70b8 RDI: 0000000000000001
> RBP: ffff88010f7b7d40 R08: ffff880126a3c4e0 R09: ffff88010f7b7ca0
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801271fd668
> R13: ffff8800966d2f78 R14: ffff88011d7089f0 R15: ffff88007dfe69a0
> FS:  00007f70441a3740(0000) GS:ffff88012a800000(0000) knlGS:00000000f77c96c0
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000002834000 CR3: 0000000107964000 CR4: 00000000000007e0
> DR0: 0000000000780000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> Stack:
>  0000000000002000 00000020810b6dde 0000000000000000 ffff88011d46db00
>  ffff8800966d3020 ffff88011d7089f0 ffff88009c7f4c10 ffff88010f7b7f2c
>  ffff88007dfe69a0 ffff88010f7b7da8 ffffffff8125cfac ffff880100000004
> Call Trace:
>  [<ffffffff8125cfac>] ext4_tmpfile+0x12c/0x180
>  [<ffffffff811cba78>] path_openat+0x238/0x700
>  [<ffffffff8100afc4>] ? native_sched_clock+0x24/0x80
>  [<ffffffff811cc647>] do_filp_open+0x47/0xa0
>  [<ffffffff811db73f>] ? __alloc_fd+0xaf/0x200
>  [<ffffffff811ba2e4>] do_sys_open+0x124/0x210
>  [<ffffffff81010725>] ? syscall_trace_enter+0x25/0x290
>  [<ffffffff811ba3ee>] SyS_open+0x1e/0x20
>  [<ffffffff816ca8d4>] tracesys+0xdd/0xe2
>  [<ffffffff81001001>] ? start_thread_common.constprop.6+0x1/0xa0
> Code: 04 00 00 00 89 04 24 31 c0 e8 c4 77 04 00 e9 43 fe ff ff 66 25 00 d0 66 3d 00 80 0f 84 0e fe ff ff 83 7b 48 00 0f 84 04 fe ff ff <0f> 0b 49 8b 8c 24 50 07 00 00 e9 88 fe ff ff 0f 1f 84 00 00 00
>
> Here we couldn't call clear_nlink() directly because in d_tmpfile() we
> will call inode_dec_link_count() to decrease ->i_nlink.  So this commit
> tries to call d_tmpfile() before ext4_orphan_add() to fix this problem.
>
> Reported-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/ext4/namei.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 234b834..35f55a0 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2316,11 +2316,11 @@ retry:
>                 inode->i_op = &ext4_file_inode_operations;
>                 inode->i_fop = &ext4_file_operations;
>                 ext4_set_aops(inode);
> +               d_tmpfile(dentry, inode);
>                 err = ext4_orphan_add(handle, inode);
>                 if (err)
>                         goto err_drop_inode;
>                 mark_inode_dirty(inode);
> -               d_tmpfile(dentry, inode);
>                 unlock_new_inode(inode);
>         }
>         if (handle)
> --
> 1.7.9.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu - July 23, 2013, 9:59 a.m.
On Tue, Jul 23, 2013 at 07:51:41AM +0000, jon ernst wrote:
> Hi Zheng,
> 
> I updated my kernel code to this commit, built kernel and loaded this kernel.
> I checked "uname -r", it is 3.11.0-rc1+ But when I run your test code,
> I always got "O_TMPFILE" undeclared.
> Could you please help, what could be wrong? (I included fcntl.h)

Ah, actually I used the following program to hit the bug.  Just for your
information.  Please let me know if you have any question.

Regards,
                                                - Zheng

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <unistd.h>
#include <sys/types.h>
#include <fcntl.h>

#define __O_TMPFILE	020000000
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)

int main(int argc, char *argv[])
{
	int fd;

	fd = open(argv[1], O_RDWR|O_TMPFILE);
	if (fd < 0) {
		perror("open ");
		return -1;
	}
	close(fd);
	return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jon ernst - July 23, 2013, 1:39 p.m.
> Ah, actually I used the following program to hit the bug.  Just for your
> information.  Please let me know if you have any question.
>
> Regards,
>                                                 - Zheng
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> #include <unistd.h>
> #include <sys/types.h>
> #include <fcntl.h>
>
> #define __O_TMPFILE     020000000
> #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
>
> int main(int argc, char *argv[])
> {
>         int fd;
>
>         fd = open(argv[1], O_RDWR|O_TMPFILE);
>         if (fd < 0) {
>                 perror("open ");
>                 return -1;
>         }
>         close(fd);
>         return 0;
> }

Thank you Zheng!  Pardon if this is a dumb question: why do we need to
manually define "O_TMPFILE"? Not like "O_APPEND" etc. ?  I saw it has
been defined in header file. (fcntl.h) Did I miss anything?

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu - July 23, 2013, 11:38 p.m.
On Tue, Jul 23, 2013 at 01:39:39PM +0000, jon ernst wrote:
> > Ah, actually I used the following program to hit the bug.  Just for your
> > information.  Please let me know if you have any question.
> >
> > Regards,
> >                                                 - Zheng
> >
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> >
> > #include <unistd.h>
> > #include <sys/types.h>
> > #include <fcntl.h>
> >
> > #define __O_TMPFILE     020000000
> > #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
> >
> > int main(int argc, char *argv[])
> > {
> >         int fd;
> >
> >         fd = open(argv[1], O_RDWR|O_TMPFILE);
> >         if (fd < 0) {
> >                 perror("open ");
> >                 return -1;
> >         }
> >         close(fd);
> >         return 0;
> > }
> 
> Thank you Zheng!  Pardon if this is a dumb question: why do we need to
> manually define "O_TMPFILE"? Not like "O_APPEND" etc. ?

My purpose is to trigger this bug and fix it.  So I manually define this
flag for my convenience.

> I saw it has
> been defined in header file. (fcntl.h) Did I miss anything?

I guess that you might 'include <fcntl.h>' header file, right?  But the
O_TMPFILE is defined in $LINUX/include/uapi/asm-generic/fcntl.h.  So
maybe compiler couldn't find this header file.

Regards,
                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jon ernst - July 24, 2013, 9:58 p.m.
> My purpose is to trigger this bug and fix it.  So I manually define this
> flag for my convenience.
>
>> I saw it has
>> been defined in header file. (fcntl.h) Did I miss anything?
>
> I guess that you might 'include <fcntl.h>' header file, right?  But the
> O_TMPFILE is defined in $LINUX/include/uapi/asm-generic/fcntl.h.  So
> maybe compiler couldn't find this header file.
>
> Regards,
>                                                 - Zheng
--
Got it. Thank you.  BTW, the weird thing is I didn't see any
discussion on ext4 mailing list before Al Viro committed this change.
The first time I heard about this is someone reporting a bug about it.
Document about this FLAG might be necessary.
Also, about this code:

/* a horrid kludge trying to make sure that this will fail on old kernels */
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)

why not doing something like this:

#ifndef O_TMPFILE
/* a horrid kludge trying to make sure that this will fail on old kernels */
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
#endif

Thanks,
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 234b834..35f55a0 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2316,11 +2316,11 @@  retry:
 		inode->i_op = &ext4_file_inode_operations;
 		inode->i_fop = &ext4_file_operations;
 		ext4_set_aops(inode);
+		d_tmpfile(dentry, inode);
 		err = ext4_orphan_add(handle, inode);
 		if (err)
 			goto err_drop_inode;
 		mark_inode_dirty(inode);
-		d_tmpfile(dentry, inode);
 		unlock_new_inode(inode);
 	}
 	if (handle)