sunrpc: fix oops in rpc_create() when the mount namespace is unshared

RFC PATCH In review Cedric Le Goater 5 participants 22 messages Sep 8, 2008 – Sep 11, 2008

Subsystems (3)

Networking NFS client NFS server

lore

Share & tools

lore

Cedric Le Goater

author

2008-09-08 13:39 UTC #0 lore raw

RFC PATCH 1/1 sunrpc: fix oops in rpc_create() when the mount namespace is unshared

On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().

The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.

Any better suggestions ?

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Pid: 1857, comm: uts-oops Not tainted (2.6.27-rc5-00319-g7686ad5 #4)
EIP: 0060:[<c024c9ab>] EFLAGS: 00210287 CPU: 0
EIP is at rpc_create+0x332/0x42f
EAX: 00000000 EBX: df26adf0 ECX: c0251887 EDX: 00000001
ESI: df26ae58 EDI: c02f293c EBP: dda0fc9c ESP: dda0fc2c
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process uts-oops (pid: 1857, ti=dda0e000 task=dd9a0778 task.ti=dda0e000)

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <[email protected]>

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Signature · 1 line

1.5.5.1

Serge E. Hallyn

2008-09-08 15:19 UTC #1 lore raw

Quoted 10 lines · Cedric Le Goater

On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?

But the utsname gets freed after the mnt_ns, so the analysis seems wrong somehow.

I trust addr2line or whatever verified that rpc_create+0x332/0x42f is exactly at the call to utsname()?

-serge

Quoted 72 lines

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Signature · 1 line

1.5.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Serge E. Hallyn

2008-09-08 15:27 UTC #2 lore raw

Quoted 6 lines · Serge E. Hallyn

Quoted 5 lines · Cedric Le Goater
On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().

Quoted 8 lines

Quoted 5 lines
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?
But the utsname gets freed after the mnt_ns, so the analysis seems wrong somehow.

(Oh, you straightened me out on irc - current->nsproxy is already NULL.)

Quoted 75 lines

I trust addr2line or whatever verified that rpc_create+0x332/0x42f is exactly at the call to utsname()?

-serge

Quoted 70 lines

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Quoted 7 lines

Quoted 2 lines
Signature · 1 line
1.5.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Cedric Le Goater

author

2008-09-08 15:37 UTC #3 lore raw

In reply to Serge E. Hallyn

Quoted 6 lines · Serge E. Hallyn

Quoted 5 lines · Cedric Le Goater
On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().

Quoted 8 lines

Quoted 5 lines
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?
But the utsname gets freed after the mnt_ns, so the analysis seems wrong somehow.

yes but switch_task_namespaces() assigns ->nsproxy to NULL. the result is the same.

Quoted 2 lines

I trust addr2line or whatever verified that rpc_create+0x332/0x42f is exactly at the call to utsname()?

yes. it points to net/sunrpc/clnt.c:216

	clnt->cl_nodelen = strlen(utsname()->nodename);

Thanks,

Quoted 72 lines

-serge

Quoted 70 lines

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Quoted 2 lines

Quoted 2 lines
Signature · 1 line
1.5.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Serge E. Hallyn

2008-09-08 15:43 UTC #4 lore raw

Quoted 19 lines · Cedric Le Goater

Quoted 6 lines · Serge E. Hallyn
Quoted 5 lines · Cedric Le Goater
On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().
Quoted 8 lines
Quoted 5 lines
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?
But the utsname gets freed after the mnt_ns, so the analysis seems wrong somehow.
yes but switch_task_namespaces() assigns ->nsproxy to NULL. the result is the same.

Quoted 6 lines

Quoted 2 lines
I trust addr2line or whatever verified that rpc_create+0x332/0x42f is exactly at the call to utsname()?
yes. it points to net/sunrpc/clnt.c:216
	clnt->cl_nodelen = strlen(utsname()->nodename);

Pavel, at the mini-summit you mentioned sunrpc transports as one example of the mini-namespaces openvz currently implements. We then apparently went off on a bit of a tangent
(http://wiki.openvz.org/Containers/Mini-summit_2008_notes#Namespaces_and_containers)
after Dave asked for a list of mini-namespaces openvz implements.

What exactly is openvz doing with sunrpc transports, and would it be a better solution to this problem?

thanks,
-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric W. Biederman

2008-09-08 16:09 UTC #5 lore raw

In reply to Cedric Le Goater

Quoted 10 lines · Cedric Le Goater

On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?

Can we push utsname into rpc_create_args and push the access of utsname up the food chain?

My gut feeling says we should capture the utsname or the uts_ns when we mount the nfs filesystem so we stay in sync for the life of the mount.

Eric

Quoted 75 lines

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f

77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +0 -0

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct

rpc_create_args *args, stru

net/sunrpc/clnt.c +2 -2

 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Signature · 2 lines

-- 
1.5.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Cedric Le Goater

author

2008-09-09 11:54 UTC #6 lore raw

Quoted 7 lines · Eric W. Biederman

Quoted 5 lines · Cedric Le Goater
On a system with nfs mounts, if a task unshares its mount namespace, a oops can occur when the system is rebooted if the task is the last to unreference the nfs mount. It will try to create a rpc request using utsname() which has been invalidated by free_nsproxy().

Quoted 8 lines

Quoted 5 lines
The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.
Any better suggestions ?
Can we push utsname into rpc_create_args and push the access of utsname up the food chain?

struct rpc_create_args seems to be used only as a stack argument for rpc_create() it's not kept in any nfs or sunrpc objects.

Quoted 3 lines

My gut feeling says we should capture the utsname or the uts_ns when we mount the nfs filesystem so we stay in sync for the life of the mount.

I see. It make sense but, looking at the code, the nfs and sunrpc will need some heavy changes ...

Thanks,

C.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Serge E. Hallyn

Acked-by on v1 1/1

2008-09-09 12:43 UTC #7 lore raw

In reply to Cedric Le Goater

Quoted 59 lines · Cedric Le Goater

The patch fixes the issue by using the global init_utsname() but at the same time, it breaks the capability of identifying rpc clients per uts namespace.

Any better suggestions ?

BUG: unable to handle kernel NULL pointer dereference at 00000004
IP: [<c024c9ab>] rpc_create+0x332/0x42f
Oops: 0000 [#1] DEBUG_PAGEALLOC

Stack: c0104532 dda0fffc dda0fcac dda0e000 dda0e000 dd93b7f0 00000009 c02f2880
       df26aefc dda0fc68 c01096b7 00000000 c0266ee0 c039a070 c039a070 dda0fc74
       c012ca67 c039a064 dda0fc8c c012cb20 c03daf74 00000011 00000000 c0275c90
Call Trace:
 [<c0104532>] ? dump_trace+0xc2/0xe2
 [<c01096b7>] ? save_stack_trace+0x1c/0x3a
 [<c012ca67>] ? save_trace+0x37/0x8c
 [<c012cb20>] ? add_lock_to_list+0x64/0x96
 [<c0256fc4>] ? rpcb_register_call+0x62/0xbb
 [<c02570c8>] ? rpcb_register+0xab/0xb3
 [<c0252f4d>] ? svc_register+0xb4/0x128
 [<c0253114>] ? svc_destroy+0xec/0x103
 [<c02531b2>] ? svc_exit_thread+0x87/0x8d
 [<c01a75cd>] ? lockd_down+0x61/0x81
 [<c01a577b>] ? nlmclnt_done+0xd/0xf
 [<c01941fe>] ? nfs_destroy_server+0x14/0x16
 [<c0194328>] ? nfs_free_server+0x4c/0xaa
 [<c019a066>] ? nfs_kill_super+0x23/0x27
 [<c0158585>] ? deactivate_super+0x3f/0x51
 [<c01695d1>] ? mntput_no_expire+0x95/0xb4
 [<c016965b>] ? release_mounts+0x6b/0x7a
 [<c01696cc>] ? __put_mnt_ns+0x62/0x70
 [<c0127501>] ? free_nsproxy+0x25/0x80
 [<c012759a>] ? switch_task_namespaces+0x3e/0x43
 [<c01275a9>] ? exit_task_namespaces+0xa/0xc
 [<c0117fed>] ? do_exit+0x4fd/0x666
 [<c01181b3>] ? do_group_exit+0x5d/0x83
 [<c011fa8c>] ? get_signal_to_deliver+0x2c8/0x2e0
 [<c0102630>] ? do_notify_resume+0x69/0x700
 [<c011d85a>] ? do_sigaction+0x134/0x145
 [<c0127205>] ? hrtimer_nanosleep+0x8f/0xce
 [<c0126d1a>] ? hrtimer_wakeup+0x0/0x1c
 [<c0103488>] ? work_notifysig+0x13/0x1b
 =======================
Code: 70 20 68 cb c1 2c c0 e8 75 4e 01 00 8b 83 ac 00 00 00 59 3d 00 f0 ff ff 5f 77 63 eb 57 a1 00 80 2d c0 8b 80 a8 02 00 00 8d 73 68 <8b> 40 04 83 c0 45 e8 41 46 f7 ff ba 20 00 00 00 83 f8 21 0f 4c
EIP: [<c024c9ab>] rpc_create+0x332/0x42f SS:ESP 0068:dda0fc2c

Quoted 1 line

Signed-off-by Cedric Le Goater <clg-NmTC/[email protected]>

Thanks, Cedric. Eric is probably right about the long-term fix, but yeah it might take a while to properly wade through the sunrpc and nfs layers to store the nodename at nfs mount time, and in the meantime this fixes a real oops.

Acked-by Serge Hallyn <serue-r/[email protected]>

Quoted 23 lines

---
 net/sunrpc/clnt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

net/sunrpc/clnt.c +2 -2

index 76739e9..a59cdf4 100644
@@ -213,10 +213,10 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru
 	}
 
 	/* save the nodename */
-	clnt->cl_nodelen = strlen(utsname()->nodename);
+	clnt->cl_nodelen = strlen(init_utsname()->nodename);
 	if (clnt->cl_nodelen > UNX_MAXNODENAME)
 		clnt->cl_nodelen = UNX_MAXNODENAME;
-	memcpy(clnt->cl_nodename, utsname()->nodename, clnt->cl_nodelen);
+	memcpy(clnt->cl_nodename, init_utsname()->nodename, clnt->cl_nodelen);
 	rpc_register_client(clnt);
 	return clnt;

Signature · 1 line

1.5.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric W. Biederman

2008-09-09 15:09 UTC #8 lore raw

Quoted 4 lines · Serge E. Hallyn

Thanks, Cedric. Eric is probably right about the long-term fix, but yeah it might take a while to properly wade through the sunrpc and nfs layers to store the nodename at nfs mount time, and in the meantime this fixes a real oops.

A very esoteric oops that hasn't shown up for two years. Please let's look at this and see what it would take to fix this properly.

What are we trying to achieve by reading utsname?

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Serge E. Hallyn

2008-09-09 15:29 UTC #9 lore raw

Quoted 8 lines · Eric W. Biederman

Quoted 4 lines · Serge E. Hallyn
Thanks, Cedric. Eric is probably right about the long-term fix, but yeah it might take a while to properly wade through the sunrpc and nfs layers to store the nodename at nfs mount time, and in the meantime this fixes a real oops.
A very esoteric oops that hasn't shown up for two years.

But an easily reproducible one.

It's not as though we'll stop looking for the right fix just bc we have this "bad" fix in for a short while.

Quoted 2 lines

Please let's look at this and see what it would take to fix this properly.

Of course. Cedric is looking at the best way to fix it...

Quoted 1 line

What are we trying to achieve by reading utsname?

It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?

I don't want to do this, but we *could* put a conditional in utsname() to have it return init_utsname if current->nsproxy is null...

-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Cedric Le Goater

author

2008-09-09 15:40 UTC #10 lore raw

Quoted 11 lines · Serge E. Hallyn

Quoted 7 lines · Eric W. Biederman
Quoted 4 lines · Serge E. Hallyn
Thanks, Cedric. Eric is probably right about the long-term fix, but yeah it might take a while to properly wade through the sunrpc and nfs layers to store the nodename at nfs mount time, and in the meantime this fixes a real oops.
A very esoteric oops that hasn't shown up for two years.
But an easily reproducible one.

Quoted 7 lines

It's not as though we'll stop looking for the right fix just bc we have this "bad" fix in for a short while.
Quoted 2 lines
Please let's look at this and see what it would take to fix this properly.
Of course. Cedric is looking at the best way to fix it...

yes. well, my eyes are making progress in the NFS code. it will take some time :)

Quoted 7 lines

Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?
I don't want to do this, but we *could* put a conditional in utsname() to have it return init_utsname if current->nsproxy is null...

I nearly did that one but it will hide future misusage of utsname(). So I think it's better to keep it that way, and let the machine oops when we need to fix our code.

C.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chuck Lever

2008-09-09 17:07 UTC #11 lore raw

In reply to Serge E. Hallyn

Quoted 12 lines · Serge E. Hallyn

Quoted 8 lines · Eric W. Biederman
Quoted 4 lines · Serge E. Hallyn
Thanks, Cedric. Eric is probably right about the long-term fix, but yeah it might take a while to properly wade through the sunrpc and nfs layers to store the nodename at nfs mount time, and in the meantime this fixes a real oops.
A very esoteric oops that hasn't shown up for two years.
But an easily reproducible one.

Quoted 7 lines

It's not as though we'll stop looking for the right fix just bc we have this "bad" fix in for a short while.
Quoted 2 lines
Please let's look at this and see what it would take to fix this properly.
Of course. Cedric is looking at the best way to fix it...

If the upper layers are responsible for providing the utsname, you will need to fix up lockd and the NFS server's callback client too, at least.

Quoted 4 lines

Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?

It appears to be used only for RPC's AUTH_SYS credentials. The nodename is used to identify the caller's host. See RFC 1831, Appendix A:

http://rfclibrary.hosting.com/rfc/rfc1831/rfc1831-16.asp

I'm not terribly familiar with uts namespaces, though. Can someone explain why we need to distinguish between these for AUTH_SYS if the caller is on a remote system?

Quoted 2 lines

I don't want to do this, but we *could* put a conditional in utsname() to have it return init_utsname if current->nsproxy is null...

I don't like the idea of an oops in here. Instead, (for now) it should warn and fail to create the client, IMO.

Signature · 6 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric W. Biederman

2008-09-09 18:20 UTC #12 lore raw

Quoted 2 lines · Chuck Lever

If the upper layers are responsible for providing the utsname, you will need to fix up lockd and the NFS server's callback client too, at least.

Actually looking at the code. It looks like a proper fix may be even simpler.
Why do we have both clnt->cl_server and clnt->cl_nodename? Or is cl_server
the other side of the connection?

Quoted 9 lines

Quoted 4 lines
Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?
It appears to be used only for RPC's AUTH_SYS credentials. The nodename is used to identify the caller's host. See RFC 1831, Appendix A:
http://rfclibrary.hosting.com/rfc/rfc1831/rfc1831-16.asp

Thanks that helps a lot.

Quoted 3 lines

I'm not terribly familiar with uts namespaces, though. Can someone explain why we need to distinguish between these for AUTH_SYS if the caller is on a remote system?

Semantically processes in different uts namespaces are on different machines.

Quoted 2 lines

I don't like the idea of an oops in here. Instead, (for now) it should warn and fail to create the client, IMO.

Which is interesting when the problem happens during NFS unmount. Although frankly it could fail anyway.

It seems strange that we are creating a client during unmount anyway.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chuck Lever

2008-09-09 19:00 UTC #13 lore raw

Quoted 11 lines · On Sep 9, 2008, at Sep 9, 2008, 2:20 PM…

Quoted 3 lines · Chuck Lever
If the upper layers are responsible for providing the utsname, you will need to
fix up lockd and the NFS server's callback client too, at least.
Actually looking at the code. It looks like a proper fix may be even simpler.
Why do we have both clnt->cl_server and clnt->cl_nodename? Or is
cl_server
the other side of the connection?

cl_server is the server-side. cl_nodename is the "machine name" of the client.

Quoted 14 lines

Quoted 11 lines
Quoted 5 lines
Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is
a part of the sunrpc spec?
It appears to be used only for RPC's AUTH_SYS credentials. The nodename is used
to identify the caller's host. See RFC 1831, Appendix A:
http://rfclibrary.hosting.com/rfc/rfc1831/rfc1831-16.asp
Thanks that helps a lot.

Quoted 8 lines

Quoted 5 lines
I'm not terribly familiar with uts namespaces, though. Can someone explain why
we need to distinguish between these for AUTH_SYS if the caller is on a remote
system?
Semantically processes in different uts namespaces are on different machines.

OK.

Quoted 9 lines

Quoted 3 lines
I don't like the idea of an oops in here. Instead, (for now) it should warn and
fail to create the client, IMO.
Which is interesting when the problem happens during NFS unmount. Although
frankly it could fail anyway.
It seems strange that we are creating a client during unmount anyway.

It's worth investigating. Just enable RPC tracing (/usr/sbin/rpcdebug -m rpc -s all) before shutting down the client.

NFS unmounting is historically difficult because during a system shutdown, unmounting is the last thing to occur, and usually happens after most system services (such as the portmapper service) have become unavailable. That's fine for local file systems, but it makes for a rather dodgy situation for NFS.

Signature · 2 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Eric W. Biederman

2008-09-09 20:08 UTC #14 lore raw

Quoted 7 lines · Chuck Lever

Quoted 6 lines · On Sep 9, 2008, at Sep 9, 2008, 2:20 PM…
Quoted 3 lines · Chuck Lever
If the upper layers are responsible for providing the utsname, you will need to
fix up lockd and the NFS server's callback client too, at least.

Quoted 5 lines

Quoted 3 lines
Actually looking at the code. It looks like a proper fix may be even simpler.
Why do we have both clnt->cl_server and clnt->cl_nodename? Or is cl_server
the other side of the connection?
cl_server is the server-side. cl_nodename is the "machine name" of the client.

Thanks, Darn. Looks like we need to capture both names at the same or a similar point.

I'm wondering if we need to capture a network namespace as well.

Quoted 9 lines

Quoted 8 lines
Quoted 3 lines
I don't like the idea of an oops in here. Instead, (for now) it should warn and
fail to create the client, IMO.
Which is interesting when the problem happens during NFS unmount. Although frankly it could fail anyway.
It seems strange that we are creating a client during unmount anyway.

Quoted 7 lines

It's worth investigating. Just enable RPC tracing (/usr/sbin/rpcdebug -m rpc -s all) before shutting down the client.
NFS unmounting is historically difficult because during a system shutdown, unmounting is the last thing to occur, and usually happens after most system services (such as the portmapper service) have become unavailable. That's fine for local file systems, but it makes for a rather dodgy situation for NFS.

Interesting.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Cedric Le Goater

author

2008-09-10 09:23 UTC #15 lore raw

In reply to Eric W. Biederman

Quoted 25 lines · Eric W. Biederman

Quoted 2 lines · Chuck Lever
If the upper layers are responsible for providing the utsname, you will need to fix up lockd and the NFS server's callback client too, at least.
Actually looking at the code. It looks like a proper fix may be even simpler.
Why do we have both clnt->cl_server and clnt->cl_nodename? Or is cl_server
the other side of the connection?
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?
It appears to be used only for RPC's AUTH_SYS credentials. The nodename is used to identify the caller's host. See RFC 1831, Appendix A:
http://rfclibrary.hosting.com/rfc/rfc1831/rfc1831-16.asp
Thanks that helps a lot.
Quoted 3 lines
I'm not terribly familiar with uts namespaces, though. Can someone explain why we need to distinguish between these for AUTH_SYS if the caller is on a remote system?
Semantically processes in different uts namespaces are on different machines.

Quoted 7 lines

Quoted 2 lines
I don't like the idea of an oops in here. Instead, (for now) it should warn and fail to create the client, IMO.
Which is interesting when the problem happens during NFS unmount. Although frankly it could fail anyway.
It seems strange that we are creating a client during unmount anyway.

the task exiting brings down the lockd thread and unregisters the lockd service with the portmapper. This is done with a rpc call which creates a client and a request.

that's how I understand the code and the oops.

C.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chuck Lever

2008-09-10 15:12 UTC #16 lore raw

Quoted 29 lines · Cedric Le Goater

Quoted 28 lines · Eric W. Biederman
Quoted 2 lines · Chuck Lever
If the upper layers are responsible for providing the utsname, you will need to fix up lockd and the NFS server's callback client too, at least.
Actually looking at the code. It looks like a proper fix may be even simpler.
Why do we have both clnt->cl_server and clnt->cl_nodename? Or is cl_server
the other side of the connection?
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
What are we trying to achieve by reading utsname?
It looks like it gets copied into the sunrpc messages so I assume it is a part of the sunrpc spec?
It appears to be used only for RPC's AUTH_SYS credentials. The nodename is used to identify the caller's host. See RFC 1831, Appendix A:
http://rfclibrary.hosting.com/rfc/rfc1831/rfc1831-16.asp
Thanks that helps a lot.
Quoted 3 lines
I'm not terribly familiar with uts namespaces, though. Can someone explain why we need to distinguish between these for AUTH_SYS if the caller is on a remote system?
Semantically processes in different uts namespaces are on different machines.
Quoted 2 lines
I don't like the idea of an oops in here. Instead, (for now) it should warn and fail to create the client, IMO.

Quoted 8 lines

Quoted 4 lines
Which is interesting when the problem happens during NFS unmount. Although frankly it could fail anyway.
It seems strange that we are creating a client during unmount anyway.
the task exiting brings down the lockd thread and unregisters the lockd service with the portmapper. This is done with a rpc call which creates a client and a request.

That makes sense.

This is likely coming from lockd_down(), and is almost certainly not coming from the same uts namespace as the lockd_up() that did the pmap_set, which was done by the first NFS mount done in the first uts namespace on the system. It's just something that the kernel has to do for maintenance.

There is only one lockd() instance that is shared among all the uts namespaces, right? In this case, what is the correct utsname to use?

Signature · 5 lines

Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric W. Biederman

2008-09-10 20:02 UTC #17 lore raw

Quoted 10 lines · Chuck Lever

That makes sense.
This is likely coming from lockd_down(), and is almost certainly not coming from the same uts namespace as the lockd_up() that did the pmap_set, which was done by the first NFS mount done in the first uts namespace on the system. It's just something that the kernel has to do for maintenance.
There is only one lockd() instance that is shared among all the uts namespaces, right? In this case, what is the correct utsname to use?

Interesting.

As a general rule I would say we should capture the uts instance in locked_up(). And use the same instance in locked_down().

I'm not at all familiar with how locked interacts with nfs mounts in a practical sense. Is there one locked instance (or at least context) per nfs mount?

The way I would expect things to work is that when we mount an nfs filesystem from an nfs server. We would create a locked context for that server, that additional nfs mounts to the same nfs server could share.

The way I would expect nfs to interact with the namespaces is for the nfs mount to capture the uts and network namespaces, and use them for all transactions relating to the mount. In particular when creating or a locked context the nfs mount would use the uts namespace and the network namespace as discriminators to see if an existing locked context is the same.

I don't think nfs has a 1-1 thread to context model which is where things get really hazy for me.

The conservative play is to always force use of the initial namespace and to deny creation of mounts that would use different namespaces. In part because the initial version of the namespace always exists. Which means as relates to Cedrics initial patch we would still need to know which mounts should cause us to use a different uts namespace so we can deny them.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chuck Lever

2008-09-10 20:54 UTC #18 lore raw

Quoted 17 lines · On Sep 10, 2008, at Sep 10, 2008, 4:02 PM…

Quoted 10 lines · Chuck Lever
That makes sense.
This is likely coming from lockd_down(), and is almost certainly not coming from the same uts namespace as the lockd_up() that did the pmap_set, which was done by the first NFS mount done in the first uts namespace on the system. It's just something that the kernel has to do for maintenance.
There is only one lockd() instance that is shared among all the uts namespaces, right? In this case, what is the correct utsname to use?
Interesting.
As a general rule I would say we should capture the uts instance in locked_up(). And use the same instance in locked_down().

Quoted 7 lines

I'm not at all familiar with how locked interacts with nfs mounts in a practical sense. Is there one locked instance (or at least context) per nfs mount?
The way I would expect things to work is that when we mount an nfs filesystem from an nfs server. We would create a locked context for that server, that additional nfs mounts to the same nfs server could share.

There is one lockd, one statd, and one rpcbind per client. These are shared between all the NFS mounts on the client. Likewise, there is one of each of these per server, and they are shared among all exports.

lockd_up() and lockd_down() maintain a count of mounts and exports, and lockd_down() shuts down lockd when the count goes to zero.

statd provides the ability to signal a server when a client reboots (and vice versa). This gives the server an indication of when to free locks for any applications on a rebooting client, and gives the client an indication of when it needs to reclaim locks on a rebooting server.

statd (user space) and lockd (kernel) have to share a cookie (mon_name) which is used to identify the client to servers, and the server to clients, so reboots can be detected. That cookie would probably need to be the initial utsname.

Quoted 3 lines

The way I would expect nfs to interact with the namespaces is for the nfs mount to capture the uts and network namespaces, and use them for all transactions relating to the mount.

That works for the main NFS protocol, perhaps, but the auxiliary protocols are another matter. They operate on behalf of a whole client or server, not on behalf of an individual mount or export.

Quoted 4 lines

In particular when creating
or a locked context the nfs mount would use the uts namespace and the network namespace as discriminators to see if an existing locked context is the same.

Possible, but I would expect this to be a lot of work for not much gain. The right answer is likely that you need a lockd and statd instance (virtual or real) for each namespace. The mounts and exports in each namespace would have their own lockd and statd.

Quoted 2 lines

I don't think nfs has a 1-1 thread to context model which is where things get really hazy for me.

Users are assigned credentials. The credentials are passed from client to server, and the server does work on behalf of that credential (user). lockd uses a credential and a process identifier to find locks on files.

AUTH_SYS credentials (the lowest common denominator) are constructed from the user's UID and GID and the client's utsname.

The kernel, then, will have to construct unique credentials for users in each uts namespace. This is likely not an NFS mount-time issue, but is instead part of the mechanism of mapping requests from processes to RPC credentials.

Quoted 6 lines

The conservative play is to always force use of the initial namespace and to deny creation of mounts that would use different namespaces. In part because the initial version of the namespace always exists. Which means as relates to Cedrics initial patch we would still need to know which mounts should cause us to use a different uts namespace so we can deny them.

OK. I think what you are saying is that NFS won't work outside of the initial uts namespace, for now?

Also, how would an automounter fit into this uts namespace scheme?

Signature · 6 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Cedric Le Goater

author

2008-09-11 09:02 UTC #19 lore raw

In reply to Eric W. Biederman

Quoted 7 lines

The conservative play is to always force use of the initial namespace and to deny creation of mounts that would use different namespaces. In part because the initial version of the namespace always exists. Which means as relates to Cedrics initial patch we would still need to know which mounts should cause us to use a different uts namespace so we can deny them.

I will send the initial patch which forces use of the initial namespace because it does fix a real oops.

Then, I should be able to find some time to work on improving the uts namespace checks when NFS mounts are done.

Thanks !

Eric W. Biederman

2008-09-11 10:27 UTC #20 lore raw

Quoted 7 lines · Cedric Le Goater

Quoted 6 lines
The conservative play is to always force use of the initial namespace and to deny creation of mounts that would use different namespaces. In part because the initial version of the namespace always exists. Which means as relates to Cedrics initial patch we would still need to know which mounts should cause us to use a different uts namespace so we can deny them.

Quoted 5 lines

I will send the initial patch which forces use of the initial namespace because it does fix a real oops.
Then, I should be able to find some time to work on improving the uts namespace checks when NFS mounts are done.

Thanks.
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Chuck Lever

2008-09-11 16:39 UTC #21 lore raw

In reply to Cedric Le Goater

Cedric-

Quoted 10 lines · Cedric Le Goater

Quoted 9 lines
The conservative play is to always force use of the initial namespace and to deny creation of mounts that would use different namespaces. In part
because the initial version of the namespace always exists. Which means
as relates to Cedrics initial patch we would still need to know which mounts should cause us to use a different uts namespace so we can deny
them.

Quoted 3 lines

I will send the initial patch which forces use of the initial namespace because
it does fix a real oops.

Today's patch looks reasonable to me. I would add a FIXME comment in net/sunrpc/clnt.c near the site of the fix.

Quoted 3 lines

Then, I should be able to find some time to work on improving the uts namespace
checks when NFS mounts are done.

As a final note, I don't think this is an issue only for NFS mounts. The problem is with how AUTH_SYS RPC credentials are formed.

cl_nodename is a copy of utsname kept in the rpc_clnt structure for efficiency. I think the auth_unix code has to be more sensitive to which utsname is "in effect" for each RPC request. We probably can't use the same utsname for all RPC requests for the life of an RPC client. And since RPC credentials are cached, we should be more careful about which cached credential is selected.

Signature · 2 lines

Chuck Lever
chuck[dot]lever[at]oracle[dot]com