linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 2.6.35.2: NFS related Oops
@ 2010-08-16 21:50 Adam Lackorzynski
  2010-08-17 10:09 ` Bian Naimeng
  0 siblings, 1 reply; 9+ messages in thread
From: Adam Lackorzynski @ 2010-08-16 21:50 UTC (permalink / raw)
  To: linux-kernel, linux-nfs

Hi,

with 2.6.35.2 I'm getting this reproducible Oops:

[  110.825396] BUG: unable to handle kernel NULL pointer dereference at (null)
[  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638] PGD be89f067 PUD bf18f067 PMD 0
[  110.828638] Oops: 0000 [#1] SMP
[  110.828638] last sysfs file: /sys/class/net/lo/operstate
[  110.828638] CPU 2
[  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base [last unloaded: scsi_wait_scan] 
[  110.828638] 
[  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
[  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
[  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX: 0000000000000000
[  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI: ffff88003bf5b9f8
[  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09: 0000000000000004
[  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12: ffff8800be258800
[  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15: ffff8800be258800
[  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063) knlGS:00000000556bd6b0
[  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
[  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4: 00000000000006e0
[  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  110.828638] Process setchecksum (pid: 11264, threadinfo ffff88003bf5a000, task ffff88003f232210)
[  110.828638] Stack:
[  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000 0000000000000ffe
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  110.828638] Call Trace:
[  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
[  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
[  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
[  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
[  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
[  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
[  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
[  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
[  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
[  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
[  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
[  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
[  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
[  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
[  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
[  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
[  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
[  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
[  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
[  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
[  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
[  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41 57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00 <8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
[  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638]  RSP <ffff88003bf5b878>
[  110.828638] CR2: 0000000000000000
[  112.840396] ---[ end trace 95282e83fd77358f ]---


Looks like arg->iap in encode_setattr() in nfs4xdr.c is 0.



Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/

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

* Re: 2.6.35.2: NFS related Oops
  2010-08-16 21:50 2.6.35.2: NFS related Oops Adam Lackorzynski
@ 2010-08-17 10:09 ` Bian Naimeng
  2010-08-17 17:14   ` Adam Lackorzynski
  0 siblings, 1 reply; 9+ messages in thread
From: Bian Naimeng @ 2010-08-17 10:09 UTC (permalink / raw)
  To: Adam Lackorzynski; +Cc: linux-kernel, linux-nfs, Trond Myklebust

> Hi,
> 
> with 2.6.35.2 I'm getting this reproducible Oops:
> 

  Please try to apply the followed patch.

  ----

   We we open a positive file just with O_EXCL but no O_CREAT, may cause kernel crash.

  Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>

---
 fs/nfs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 29539ce..1a672dd 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1100,7 +1100,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
 		goto no_open_dput;
 	openflags = nd->intent.open.flags;
 	/* We cannot do exclusive creation on a positive dentry */
-	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
+	if (openflags & O_EXCL)
 		goto no_open_dput;
 	/* We can't create new files, or truncate existing ones here */
 	openflags &= ~(O_CREAT|O_TRUNC);
-- 
1.7.0



> [  110.825396] BUG: unable to handle kernel NULL pointer dereference at (null)
> [  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638] PGD be89f067 PUD bf18f067 PMD 0
> [  110.828638] Oops: 0000 [#1] SMP
> [  110.828638] last sysfs file: /sys/class/net/lo/operstate
> [  110.828638] CPU 2
> [  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base [last unloaded: scsi_wait_scan] 
> [  110.828638] 
> [  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
> [  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
> [  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX: 0000000000000000
> [  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI: ffff88003bf5b9f8
> [  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09: 0000000000000004
> [  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12: ffff8800be258800
> [  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15: ffff8800be258800
> [  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063) knlGS:00000000556bd6b0
> [  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
> [  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4: 00000000000006e0
> [  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [  110.828638] Process setchecksum (pid: 11264, threadinfo ffff88003bf5a000, task ffff88003f232210)
> [  110.828638] Stack:
> [  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000 0000000000000ffe
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [  110.828638] Call Trace:
> [  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
> [  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
> [  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
> [  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
> [  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
> [  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
> [  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
> [  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
> [  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
> [  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
> [  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
> [  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
> [  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
> [  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
> [  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
> [  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
> [  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
> [  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
> [  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
> [  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
> [  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
> [  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41 57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00 <8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
> [  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638]  RSP <ffff88003bf5b878>
> [  110.828638] CR2: 0000000000000000
> [  112.840396] ---[ end trace 95282e83fd77358f ]---
> 
> 
> Looks like arg->iap in encode_setattr() in nfs4xdr.c is 0.
> 
> 
> 
> Adam

-- 
Regards
Bian Naimeng


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

* Re: 2.6.35.2: NFS related Oops
  2010-08-17 10:09 ` Bian Naimeng
@ 2010-08-17 17:14   ` Adam Lackorzynski
  2010-08-17 22:43     ` Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Adam Lackorzynski @ 2010-08-17 17:14 UTC (permalink / raw)
  To: Bian Naimeng; +Cc: linux-kernel, linux-nfs, Trond Myklebust, stable


On Tue Aug 17, 2010 at 18:09:53 +0800, Bian Naimeng wrote:
>   Please try to apply the followed patch.

Thanks, this fixes the Oops. Patch is required for both 2.6.35 and
2.6.36 trees.

 
----
 
We we open a positive file just with O_EXCL but no O_CREAT, may cause kernel crash.

Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>
 
---
 fs/nfs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 29539ce..1a672dd 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1100,7 +1100,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
 		goto no_open_dput;
 	openflags = nd->intent.open.flags;
 	/* We cannot do exclusive creation on a positive dentry */
-	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
+	if (openflags & O_EXCL)
 		goto no_open_dput;
 	/* We can't create new files, or truncate existing ones here */
 	openflags &= ~(O_CREAT|O_TRUNC);
-- 
1.7.0



Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/

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

* Re: 2.6.35.2: NFS related Oops
  2010-08-17 17:14   ` Adam Lackorzynski
@ 2010-08-17 22:43     ` Trond Myklebust
  2010-08-18  2:12       ` Bian Naimeng
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2010-08-17 22:43 UTC (permalink / raw)
  To: Adam Lackorzynski; +Cc: Bian Naimeng, linux-kernel, linux-nfs, stable

On Tue, 2010-08-17 at 19:14 +0200, Adam Lackorzynski wrote:
> On Tue Aug 17, 2010 at 18:09:53 +0800, Bian Naimeng wrote:
> >   Please try to apply the followed patch.
> 
> Thanks, this fixes the Oops. Patch is required for both 2.6.35 and
> 2.6.36 trees.
> 
>  
> ----
>  
> We we open a positive file just with O_EXCL but no O_CREAT, may cause kernel crash.
> 
> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>
>  
> ---
>  fs/nfs/dir.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>  
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 29539ce..1a672dd 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1100,7 +1100,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
>  		goto no_open_dput;
>  	openflags = nd->intent.open.flags;
>  	/* We cannot do exclusive creation on a positive dentry */
> -	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
> +	if (openflags & O_EXCL)
>  		goto no_open_dput;
>  	/* We can't create new files, or truncate existing ones here */
>  	openflags &= ~(O_CREAT|O_TRUNC);
> -- 

Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
for whether or not we're doing an exclusive create.

Does the following patch work?

Cheers
  Trond
-----------------------------------------------------------------------------------------------
NFS: Fix an Oops in the NFSv4 atomic open code

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Adam Lackorzynski reports:

with 2.6.35.2 I'm getting this reproducible Oops:

[  110.825396] BUG: unable to handle kernel NULL pointer dereference at
(null)
[  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638] PGD be89f067 PUD bf18f067 PMD 0
[  110.828638] Oops: 0000 [#1] SMP
[  110.828638] last sysfs file: /sys/class/net/lo/operstate
[  110.828638] CPU 2
[  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod
i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot
sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base
[last unloaded: scsi_wait_scan] 
[  110.828638] 
[  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
[  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>]
encode_attrs+0x1a/0x2a4
[  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
[  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX:
0000000000000000
[  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI:
ffff88003bf5b9f8
[  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09:
0000000000000004
[  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12:
ffff8800be258800
[  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15:
ffff8800be258800
[  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063)
knlGS:00000000556bd6b0
[  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
[  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4:
00000000000006e0
[  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[  110.828638] Process setchecksum (pid: 11264, threadinfo
ffff88003bf5a000, task ffff88003f232210)
[  110.828638] Stack:
[  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000
0000000000000ffe
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[  110.828638] Call Trace:
[  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
[  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
[  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
[  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
[  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
[  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
[  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
[  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
[  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
[  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
[  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
[  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
[  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
[  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
[  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
[  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
[  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
[  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
[  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
[  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
[  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
[  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41
57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00
<8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
[  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638]  RSP <ffff88003bf5b878>
[  110.828638] CR2: 0000000000000000
[  112.840396] ---[ end trace 95282e83fd77358f ]---

It looks as if Al Viro's commit 3516586a424ea5727be089da6541cbd5644f0497
(make O_EXCL in nd->intent.flags visible in nd->flags) missed a case.

Cc: stable@kernel.org
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index bd91b27..275efac 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1107,7 +1107,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
 		goto no_open_dput;
 	openflags = nd->intent.open.flags;
 	/* We cannot do exclusive creation on a positive dentry */
-	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
+	if (nd->flags & LOOKUP_EXCL)
 		goto no_open_dput;
 	/* We can't create new files, or truncate existing ones here */
 	openflags &= ~(O_CREAT|O_TRUNC);


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

* Re: 2.6.35.2: NFS related Oops
  2010-08-17 22:43     ` Trond Myklebust
@ 2010-08-18  2:12       ` Bian Naimeng
  2010-08-18  2:49         ` Bian Naimeng
  0 siblings, 1 reply; 9+ messages in thread
From: Bian Naimeng @ 2010-08-18  2:12 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Adam Lackorzynski, linux-kernel, linux-nfs, stable

> On Tue, 2010-08-17 at 19:14 +0200, Adam Lackorzynski wrote:
>> On Tue Aug 17, 2010 at 18:09:53 +0800, Bian Naimeng wrote:
>>>   Please try to apply the followed patch.
>> Thanks, this fixes the Oops. Patch is required for both 2.6.35 and
>> 2.6.36 trees.

  ... snip ...

>>  	/* We can't create new files, or truncate existing ones here */
>>  	openflags &= ~(O_CREAT|O_TRUNC);
>> -- 
> 
> Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
> for whether or not we're doing an exclusive create.
> 
> Does the following patch work?
> 

  Hi Trond, i guess it's not work.

  As i see, if we want get LOOKUP_EXCL at nd->flags,  we must open file with 
  O_CREAT and O_EXCL, "nd->flags & LOOKUP_EXCL" have the same effect with
  "(openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)", so i think the kernel
  still crash, right?

-- 
Regards
Bian Naimeng

> -----------------------------------------------------------------------------------------------
> NFS: Fix an Oops in the NFSv4 atomic open code
> 
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Adam Lackorzynski reports:
> 
> with 2.6.35.2 I'm getting this reproducible Oops:
> 
> [  110.825396] BUG: unable to handle kernel NULL pointer dereference at
> (null)
> [  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638] PGD be89f067 PUD bf18f067 PMD 0
> [  110.828638] Oops: 0000 [#1] SMP
> [  110.828638] last sysfs file: /sys/class/net/lo/operstate
> [  110.828638] CPU 2
> [  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod
> i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot
> sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base
> [last unloaded: scsi_wait_scan] 
> [  110.828638] 
> [  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
> [  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>]
> encode_attrs+0x1a/0x2a4
> [  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
> [  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX:
> 0000000000000000
> [  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI:
> ffff88003bf5b9f8
> [  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09:
> 0000000000000004
> [  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12:
> ffff8800be258800
> [  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15:
> ffff8800be258800
> [  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063)
> knlGS:00000000556bd6b0
> [  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
> [  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4:
> 00000000000006e0
> [  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [  110.828638] Process setchecksum (pid: 11264, threadinfo
> ffff88003bf5a000, task ffff88003f232210)
> [  110.828638] Stack:
> [  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000
> 0000000000000ffe
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
> 0000000000000000
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
> 0000000000000000
> [  110.828638] Call Trace:
> [  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
> [  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
> [  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
> [  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
> [  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
> [  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
> [  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
> [  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
> [  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
> [  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
> [  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
> [  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
> [  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
> [  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
> [  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
> [  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
> [  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
> [  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
> [  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
> [  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
> [  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
> [  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41
> 57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00
> <8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
> [  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638]  RSP <ffff88003bf5b878>
> [  110.828638] CR2: 0000000000000000
> [  112.840396] ---[ end trace 95282e83fd77358f ]---
> 
> It looks as if Al Viro's commit 3516586a424ea5727be089da6541cbd5644f0497
> (make O_EXCL in nd->intent.flags visible in nd->flags) missed a case.
> 
> Cc: stable@kernel.org
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfs/dir.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index bd91b27..275efac 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1107,7 +1107,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
>  		goto no_open_dput;
>  	openflags = nd->intent.open.flags;
>  	/* We cannot do exclusive creation on a positive dentry */
> -	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
> +	if (nd->flags & LOOKUP_EXCL)
>  		goto no_open_dput;
>  	/* We can't create new files, or truncate existing ones here */
>  	openflags &= ~(O_CREAT|O_TRUNC);
> 


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

* Re: 2.6.35.2: NFS related Oops
  2010-08-18  2:12       ` Bian Naimeng
@ 2010-08-18  2:49         ` Bian Naimeng
  2010-08-18 11:36           ` Adam Lackorzynski
  0 siblings, 1 reply; 9+ messages in thread
From: Bian Naimeng @ 2010-08-18  2:49 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Adam Lackorzynski, linux-kernel, linux-nfs, stable

>>>  	/* We can't create new files, or truncate existing ones here */
>>>  	openflags &= ~(O_CREAT|O_TRUNC);
>>> -- 
>> Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
>> for whether or not we're doing an exclusive create.
>>
>> Does the following patch work?
>>
> 
>   Hi Trond, i guess it's not work.
> 
>   As i see, if we want get LOOKUP_EXCL at nd->flags,  we must open file with 
>   O_CREAT and O_EXCL, "nd->flags & LOOKUP_EXCL" have the same effect with
>   "(openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)", so i think the kernel
>   still crash, right?
> 

What about this one?

-- 
Regards
Bian Naimeng

-------------------------------------------------------------------------------------

   We we open a positive file just with O_EXCL but no O_CREAT, may cause kernel crash.

  Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>

---
 fs/namei.c   |    7 +++----
 fs/nfs/dir.c |    2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 17ea76b..6680a38 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1813,11 +1813,10 @@ reval:
 	nd.intent.open.create_mode = mode;
 	nd.flags &= ~LOOKUP_PARENT;
 	nd.flags |= LOOKUP_OPEN;
-	if (open_flag & O_CREAT) {
+	if (open_flag & O_CREAT)
 		nd.flags |= LOOKUP_CREATE;
-		if (open_flag & O_EXCL)
-			nd.flags |= LOOKUP_EXCL;
-	}
+	if (open_flag & O_EXCL)
+		nd.flags |= LOOKUP_EXCL;
 	if (open_flag & O_DIRECTORY)
 		nd.flags |= LOOKUP_DIRECTORY;
 	if (!(open_flag & O_NOFOLLOW))
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 29539ce..bc25da9 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1100,7 +1100,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
 		goto no_open_dput;
 	openflags = nd->intent.open.flags;
 	/* We cannot do exclusive creation on a positive dentry */
-	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
+	if (nd->flags & LOOKUP_EXCL)
 		goto no_open_dput;
 	/* We can't create new files, or truncate existing ones here */
 	openflags &= ~(O_CREAT|O_TRUNC);
-- 
1.7.0




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

* Re: 2.6.35.2: NFS related Oops
  2010-08-18  2:49         ` Bian Naimeng
@ 2010-08-18 11:36           ` Adam Lackorzynski
  2010-08-18 13:36             ` Trond Myklebust
  0 siblings, 1 reply; 9+ messages in thread
From: Adam Lackorzynski @ 2010-08-18 11:36 UTC (permalink / raw)
  To: Bian Naimeng; +Cc: Trond Myklebust, linux-kernel, linux-nfs, stable


On Wed Aug 18, 2010 at 10:49:04 +0800, Bian Naimeng wrote:
> >>>  	/* We can't create new files, or truncate existing ones here */
> >>>  	openflags &= ~(O_CREAT|O_TRUNC);
> >>> -- 
> >> Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
> >> for whether or not we're doing an exclusive create.
> >>
> >> Does the following patch work?
> >>
> > 
> >   Hi Trond, i guess it's not work.
> > 
> >   As i see, if we want get LOOKUP_EXCL at nd->flags,  we must open file with 
> >   O_CREAT and O_EXCL, "nd->flags & LOOKUP_EXCL" have the same effect with
> >   "(openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)", so i think the kernel
> >   still crash, right?

I can confirm, it's oopsing.

> What about this one?

This one works.

> 
>    We we open a positive file just with O_EXCL but no O_CREAT, may cause kernel crash.
> 
>   Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>
> 
> ---
>  fs/namei.c   |    7 +++----
>  fs/nfs/dir.c |    2 +-
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 17ea76b..6680a38 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1813,11 +1813,10 @@ reval:
>  	nd.intent.open.create_mode = mode;
>  	nd.flags &= ~LOOKUP_PARENT;
>  	nd.flags |= LOOKUP_OPEN;
> -	if (open_flag & O_CREAT) {
> +	if (open_flag & O_CREAT)
>  		nd.flags |= LOOKUP_CREATE;
> -		if (open_flag & O_EXCL)
> -			nd.flags |= LOOKUP_EXCL;
> -	}
> +	if (open_flag & O_EXCL)
> +		nd.flags |= LOOKUP_EXCL;
>  	if (open_flag & O_DIRECTORY)
>  		nd.flags |= LOOKUP_DIRECTORY;
>  	if (!(open_flag & O_NOFOLLOW))
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 29539ce..bc25da9 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1100,7 +1100,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
>  		goto no_open_dput;
>  	openflags = nd->intent.open.flags;
>  	/* We cannot do exclusive creation on a positive dentry */
> -	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
> +	if (nd->flags & LOOKUP_EXCL)
>  		goto no_open_dput;
>  	/* We can't create new files, or truncate existing ones here */
>  	openflags &= ~(O_CREAT|O_TRUNC);
> -- 
> 1.7.0

Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/

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

* Re: 2.6.35.2: NFS related Oops
  2010-08-18 11:36           ` Adam Lackorzynski
@ 2010-08-18 13:36             ` Trond Myklebust
  2010-08-18 15:44               ` Adam Lackorzynski
  0 siblings, 1 reply; 9+ messages in thread
From: Trond Myklebust @ 2010-08-18 13:36 UTC (permalink / raw)
  To: Adam Lackorzynski; +Cc: Bian Naimeng, linux-kernel, linux-nfs, stable

On Wed, 2010-08-18 at 13:36 +0200, Adam Lackorzynski wrote:
> On Wed Aug 18, 2010 at 10:49:04 +0800, Bian Naimeng wrote:
> > >>>  	/* We can't create new files, or truncate existing ones here */
> > >>>  	openflags &= ~(O_CREAT|O_TRUNC);
> > >>> -- 
> > >> Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
> > >> for whether or not we're doing an exclusive create.
> > >>
> > >> Does the following patch work?
> > >>
> > > 
> > >   Hi Trond, i guess it's not work.
> > > 
> > >   As i see, if we want get LOOKUP_EXCL at nd->flags,  we must open file with 
> > >   O_CREAT and O_EXCL, "nd->flags & LOOKUP_EXCL" have the same effect with
> > >   "(openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)", so i think the kernel
> > >   still crash, right?
> 
> I can confirm, it's oopsing.
> 
> > What about this one?
> 
> This one works.

OK... Let's just do the right thing in the NFS layer. open(O_EXCL)
without an O_CREAT is treated as an ordinary open() by most filesystems
(including NFSv3), so let's fix NFSv4 to do that too...

Cheers
  Trond
---------------------------------------------------------------------------------------------------
NFS: Fix an Oops in the NFSv4 atomic open code

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Adam Lackorzynski reports:

with 2.6.35.2 I'm getting this reproducible Oops:

[  110.825396] BUG: unable to handle kernel NULL pointer dereference at
(null)
[  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638] PGD be89f067 PUD bf18f067 PMD 0
[  110.828638] Oops: 0000 [#1] SMP
[  110.828638] last sysfs file: /sys/class/net/lo/operstate
[  110.828638] CPU 2
[  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod
i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot
sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base
[last unloaded: scsi_wait_scan]
[  110.828638]
[  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
[  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>]
encode_attrs+0x1a/0x2a4
[  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
[  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX:
0000000000000000
[  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI:
ffff88003bf5b9f8
[  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09:
0000000000000004
[  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12:
ffff8800be258800
[  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15:
ffff8800be258800
[  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063)
knlGS:00000000556bd6b0
[  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
[  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4:
00000000000006e0
[  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[  110.828638] Process setchecksum (pid: 11264, threadinfo
ffff88003bf5a000, task ffff88003f232210)
[  110.828638] Stack:
[  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000
0000000000000ffe
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[  110.828638] Call Trace:
[  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
[  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
[  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
[  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
[  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
[  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
[  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
[  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
[  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
[  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
[  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
[  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
[  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
[  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
[  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
[  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
[  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
[  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
[  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
[  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
[  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
[  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41
57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00
<8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
[  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
[  110.828638]  RSP <ffff88003bf5b878>
[  110.828638] CR2: 0000000000000000
[  112.840396] ---[ end trace 95282e83fd77358f ]---

We need to ensure that the O_EXCL flag is turned off if the user doesn't
set O_CREAT.

Cc: stable@kernel.org
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c      |    2 +-
 fs/nfs/nfs4proc.c |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)


diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index bd91b27..e257172 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1110,7 +1110,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
 	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
 		goto no_open_dput;
 	/* We can't create new files, or truncate existing ones here */
-	openflags &= ~(O_CREAT|O_TRUNC);
+	openflags &= ~(O_CREAT|O_EXCL|O_TRUNC);
 
 	/*
 	 * Note: we're not holding inode->i_mutex and so may be racing with
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6b44bbf..089da5b 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2036,7 +2036,8 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 	struct rpc_cred *cred;
 	struct nfs4_state *state;
 	struct dentry *res;
-	fmode_t fmode = nd->intent.open.flags & (FMODE_READ | FMODE_WRITE | FMODE_EXEC);
+	int open_flags = nd->intent.open.flags;
+	fmode_t fmode = open_flags & (FMODE_READ | FMODE_WRITE | FMODE_EXEC);
 
 	if (nd->flags & LOOKUP_CREATE) {
 		attr.ia_mode = nd->intent.open.create_mode;
@@ -2044,8 +2045,9 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 		if (!IS_POSIXACL(dir))
 			attr.ia_mode &= ~current_umask();
 	} else {
+		open_flags &= ~O_EXCL;
 		attr.ia_valid = 0;
-		BUG_ON(nd->intent.open.flags & O_CREAT);
+		BUG_ON(open_flags & O_CREAT);
 	}
 
 	cred = rpc_lookup_cred();
@@ -2054,7 +2056,7 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 	parent = dentry->d_parent;
 	/* Protect against concurrent sillydeletes */
 	nfs_block_sillyrename(parent);
-	state = nfs4_do_open(dir, &path, fmode, nd->intent.open.flags, &attr, cred);
+	state = nfs4_do_open(dir, &path, fmode, open_flags, &attr, cred);
 	put_rpccred(cred);
 	if (IS_ERR(state)) {
 		if (PTR_ERR(state) == -ENOENT) {


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

* Re: 2.6.35.2: NFS related Oops
  2010-08-18 13:36             ` Trond Myklebust
@ 2010-08-18 15:44               ` Adam Lackorzynski
  0 siblings, 0 replies; 9+ messages in thread
From: Adam Lackorzynski @ 2010-08-18 15:44 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Bian Naimeng, linux-kernel, linux-nfs, stable


On Wed Aug 18, 2010 at 09:36:33 -0400, Trond Myklebust wrote:
> On Wed, 2010-08-18 at 13:36 +0200, Adam Lackorzynski wrote:
> > On Wed Aug 18, 2010 at 10:49:04 +0800, Bian Naimeng wrote:
> > > >>>  	/* We can't create new files, or truncate existing ones here */
> > > >>>  	openflags &= ~(O_CREAT|O_TRUNC);
> > > >>> -- 
> > > >> Nope. The problem is the recent switch to LOOKUP_EXCL as the authority
> > > >> for whether or not we're doing an exclusive create.
> > > >>
> > > >> Does the following patch work?
> > > >>
> > > > 
> > > >   Hi Trond, i guess it's not work.
> > > > 
> > > >   As i see, if we want get LOOKUP_EXCL at nd->flags,  we must open file with 
> > > >   O_CREAT and O_EXCL, "nd->flags & LOOKUP_EXCL" have the same effect with
> > > >   "(openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL)", so i think the kernel
> > > >   still crash, right?
> > 
> > I can confirm, it's oopsing.
> > 
> > > What about this one?
> > 
> > This one works.
> 
> OK... Let's just do the right thing in the NFS layer. open(O_EXCL)
> without an O_CREAT is treated as an ordinary open() by most filesystems
> (including NFSv3), so let's fix NFSv4 to do that too...

Thanks, I can confirm the Oops is gone.

> ---------------------------------------------------------------------------------------------------
> NFS: Fix an Oops in the NFSv4 atomic open code
> 
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Adam Lackorzynski reports:
> 
> with 2.6.35.2 I'm getting this reproducible Oops:
> 
> [  110.825396] BUG: unable to handle kernel NULL pointer dereference at
> (null)
> [  110.828638] IP: [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638] PGD be89f067 PUD bf18f067 PMD 0
> [  110.828638] Oops: 0000 [#1] SMP
> [  110.828638] last sysfs file: /sys/class/net/lo/operstate
> [  110.828638] CPU 2
> [  110.828638] Modules linked in: rtc_cmos rtc_core rtc_lib amd64_edac_mod
> i2c_amd756 edac_core i2c_core dm_mirror dm_region_hash dm_log dm_snapshot
> sg sr_mod usb_storage ohci_hcd mptspi tg3 mptscsih mptbase usbcore nls_base
> [last unloaded: scsi_wait_scan]
> [  110.828638]
> [  110.828638] Pid: 11264, comm: setchecksum Not tainted 2.6.35.2 #1
> [  110.828638] RIP: 0010:[<ffffffff811247b7>]  [<ffffffff811247b7>]
> encode_attrs+0x1a/0x2a4
> [  110.828638] RSP: 0000:ffff88003bf5b878  EFLAGS: 00010296
> [  110.828638] RAX: ffff8800bddb48a8 RBX: ffff88003bf5bb18 RCX:
> 0000000000000000
> [  110.828638] RDX: ffff8800be258800 RSI: 0000000000000000 RDI:
> ffff88003bf5b9f8
> [  110.828638] RBP: 0000000000000000 R08: ffff8800bddb48a8 R09:
> 0000000000000004
> [  110.828638] R10: 0000000000000003 R11: ffff8800be779000 R12:
> ffff8800be258800
> [  110.828638] R13: ffff88003bf5b9f8 R14: ffff88003bf5bb20 R15:
> ffff8800be258800
> [  110.828638] FS:  0000000000000000(0000) GS:ffff880041e00000(0063)
> knlGS:00000000556bd6b0
> [  110.828638] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
> [  110.828638] CR2: 0000000000000000 CR3: 00000000be8ef000 CR4:
> 00000000000006e0
> [  110.828638] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [  110.828638] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [  110.828638] Process setchecksum (pid: 11264, threadinfo
> ffff88003bf5a000, task ffff88003f232210)
> [  110.828638] Stack:
> [  110.828638]  0000000000000000 ffff8800bfbcf920 0000000000000000
> 0000000000000ffe
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
> 0000000000000000
> [  110.828638] <0> 0000000000000000 0000000000000000 0000000000000000
> 0000000000000000
> [  110.828638] Call Trace:
> [  110.828638]  [<ffffffff81124c1f>] ? nfs4_xdr_enc_setattr+0x90/0xb4
> [  110.828638]  [<ffffffff81371161>] ? call_transmit+0x1c3/0x24a
> [  110.828638]  [<ffffffff813774d9>] ? __rpc_execute+0x78/0x22a
> [  110.828638]  [<ffffffff81371a91>] ? rpc_run_task+0x21/0x2b
> [  110.828638]  [<ffffffff81371b7e>] ? rpc_call_sync+0x3d/0x5d
> [  110.828638]  [<ffffffff8111e284>] ? _nfs4_do_setattr+0x11b/0x147
> [  110.828638]  [<ffffffff81109466>] ? nfs_init_locked+0x0/0x32
> [  110.828638]  [<ffffffff810ac521>] ? ifind+0x4e/0x90
> [  110.828638]  [<ffffffff8111e2fb>] ? nfs4_do_setattr+0x4b/0x6e
> [  110.828638]  [<ffffffff8111e634>] ? nfs4_do_open+0x291/0x3a6
> [  110.828638]  [<ffffffff8111ed81>] ? nfs4_open_revalidate+0x63/0x14a
> [  110.828638]  [<ffffffff811056c4>] ? nfs_open_revalidate+0xd7/0x161
> [  110.828638]  [<ffffffff810a2de4>] ? do_lookup+0x1a4/0x201
> [  110.828638]  [<ffffffff810a4733>] ? link_path_walk+0x6a/0x9d5
> [  110.828638]  [<ffffffff810a42b6>] ? do_last+0x17b/0x58e
> [  110.828638]  [<ffffffff810a5fbe>] ? do_filp_open+0x1bd/0x56e
> [  110.828638]  [<ffffffff811cd5e0>] ? _atomic_dec_and_lock+0x30/0x48
> [  110.828638]  [<ffffffff810a9b1b>] ? dput+0x37/0x152
> [  110.828638]  [<ffffffff810ae063>] ? alloc_fd+0x69/0x10a
> [  110.828638]  [<ffffffff81099f39>] ? do_sys_open+0x56/0x100
> [  110.828638]  [<ffffffff81027a22>] ? ia32_sysret+0x0/0x5
> [  110.828638] Code: 83 f1 01 e8 f5 ca ff ff 48 83 c4 50 5b 5d 41 5c c3 41
> 57 41 56 41 55 49 89 fd 41 54 49 89 d4 55 48 89 f5 53 48 81 ec 18 01 00 00
> <8b> 06 89 c2 83 e2 08 83 fa 01 19 db 83 e3 f8 83 c3 18 a8 01 8d
> [  110.828638] RIP  [<ffffffff811247b7>] encode_attrs+0x1a/0x2a4
> [  110.828638]  RSP <ffff88003bf5b878>
> [  110.828638] CR2: 0000000000000000
> [  112.840396] ---[ end trace 95282e83fd77358f ]---
> 
> We need to ensure that the O_EXCL flag is turned off if the user doesn't
> set O_CREAT.
> 
> Cc: stable@kernel.org
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfs/dir.c      |    2 +-
>  fs/nfs/nfs4proc.c |    8 +++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index bd91b27..e257172 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1110,7 +1110,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
>  	if ((openflags & (O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL))
>  		goto no_open_dput;
>  	/* We can't create new files, or truncate existing ones here */
> -	openflags &= ~(O_CREAT|O_TRUNC);
> +	openflags &= ~(O_CREAT|O_EXCL|O_TRUNC);
>  
>  	/*
>  	 * Note: we're not holding inode->i_mutex and so may be racing with
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6b44bbf..089da5b 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2036,7 +2036,8 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
>  	struct rpc_cred *cred;
>  	struct nfs4_state *state;
>  	struct dentry *res;
> -	fmode_t fmode = nd->intent.open.flags & (FMODE_READ | FMODE_WRITE | FMODE_EXEC);
> +	int open_flags = nd->intent.open.flags;
> +	fmode_t fmode = open_flags & (FMODE_READ | FMODE_WRITE | FMODE_EXEC);
>  
>  	if (nd->flags & LOOKUP_CREATE) {
>  		attr.ia_mode = nd->intent.open.create_mode;
> @@ -2044,8 +2045,9 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
>  		if (!IS_POSIXACL(dir))
>  			attr.ia_mode &= ~current_umask();
>  	} else {
> +		open_flags &= ~O_EXCL;
>  		attr.ia_valid = 0;
> -		BUG_ON(nd->intent.open.flags & O_CREAT);
> +		BUG_ON(open_flags & O_CREAT);
>  	}
>  
>  	cred = rpc_lookup_cred();
> @@ -2054,7 +2056,7 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
>  	parent = dentry->d_parent;
>  	/* Protect against concurrent sillydeletes */
>  	nfs_block_sillyrename(parent);
> -	state = nfs4_do_open(dir, &path, fmode, nd->intent.open.flags, &attr, cred);
> +	state = nfs4_do_open(dir, &path, fmode, open_flags, &attr, cred);
>  	put_rpccred(cred);
>  	if (IS_ERR(state)) {
>  		if (PTR_ERR(state) == -ENOENT) {
> 

Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/

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

end of thread, other threads:[~2010-08-18 15:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-16 21:50 2.6.35.2: NFS related Oops Adam Lackorzynski
2010-08-17 10:09 ` Bian Naimeng
2010-08-17 17:14   ` Adam Lackorzynski
2010-08-17 22:43     ` Trond Myklebust
2010-08-18  2:12       ` Bian Naimeng
2010-08-18  2:49         ` Bian Naimeng
2010-08-18 11:36           ` Adam Lackorzynski
2010-08-18 13:36             ` Trond Myklebust
2010-08-18 15:44               ` Adam Lackorzynski

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).