[PATCH 0/2] Enable the in-kernel id mapping to be on by default.

[PATCH 0/2] Enable the in-kernel id mapping to be on by default.

[Steve Dickson]

This patch set enables the in-kernel id mapping upcall
mechanism, which will fix deadlocks that can occur 
between the rpc.idmapd daemon and the kernel. 

This set also correct the binary name in the kernel 
documentation

Steve Dickson (2):
  Enable the in-kernel ID mapping upcall mechanism
  Renamed the binary that handles the upcalls to /usr/sbin/nfsidmap

 Documentation/filesystems/nfs/idmapper.txt |   22 +++++++++++-----------
 fs/nfs/Kconfig                             |   10 ++--------
 2 files changed, 13 insertions(+), 19 deletions(-)

-- 
1.7.6.4

[PATCH 1/2] Enable the in-kernel ID mapping upcall mechanism

[Steve Dickson]

To solve deadlocks that can occur between the rpc.idmapd daemon
and the kernel, enables the in-kernel upcall mechanism that 
uses the key rings to start the user/group ids.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 fs/nfs/Kconfig |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index dbcd821..94ad028 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -134,12 +134,6 @@ config NFS_USE_KERNEL_DNS
 	default y
 
 config NFS_USE_NEW_IDMAPPER
-	bool "Use the new idmapper upcall routine"
+	bool "Use the idmapper upcall routine"
 	depends on NFS_V4 && KEYS
-	help
-	  Say Y here if you want NFS to use the new idmapper upcall functions.
-	  You will need /sbin/request-key (usually provided by the keyutils
-	  package).  For details, read
-	  <file:Documentation/filesystems/nfs/idmapper.txt>.
-
-	  If you are unsure, say N.
+	default y
-- 
1.7.6.4

[PATCH 2/2] idmapper.txt: Fix the upcall binary name.

[Steve Dickson]

The binary that handles the upcalls from is kernel is called
nfsidmap not nfs.idmap.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 Documentation/filesystems/nfs/idmapper.txt |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/filesystems/nfs/idmapper.txt b/Documentation/filesystems/nfs/idmapper.txt
index 120fd3c..9c1925a 100644
--- a/Documentation/filesystems/nfs/idmapper.txt
+++ b/Documentation/filesystems/nfs/idmapper.txt
@@ -6,7 +6,7 @@ Id mapper is used by NFS to translate user and group ids into names, and to
 translate user and group names into ids.  Part of this translation involves
 performing an upcall to userspace to request the information.  Id mapper will
 user request-key to perform this upcall and cache the result.  The program
-/usr/sbin/nfs.idmap should be called by request-key, and will perform the
+/usr/sbin/nfsidmap should be called by request-key, and will perform the
 translation and initialize a key with the resulting information.
 
  NFS_USE_NEW_IDMAPPER must be selected when configuring the kernel to use this
@@ -20,12 +20,12 @@ direct the upcall.  The following line should be added:
 
 #OP	TYPE	DESCRIPTION	CALLOUT INFO	PROGRAM ARG1 ARG2 ARG3 ...
 #======	=======	===============	===============	===============================
-create	id_resolver	*	*		/usr/sbin/nfs.idmap %k %d 600
+create	id_resolver	*	*		/usr/sbin/nfsidmap %k %d 600
 
-This will direct all id_resolver requests to the program /usr/sbin/nfs.idmap.
+This will direct all id_resolver requests to the program /usr/sbin/nfsidmap.
 The last parameter, 600, defines how many seconds into the future the key will
-expire.  This parameter is optional for /usr/sbin/nfs.idmap.  When the timeout
-is not specified, nfs.idmap will default to 600 seconds.
+expire.  This parameter is optional for /usr/sbin/nfsidmap.  When the timeout
+is not specified, nfsidmap will default to 600 seconds.
 
 id mapper uses for key descriptions:
 	  uid:  Find the UID for the given user
@@ -40,28 +40,28 @@ would edit your request-key.conf so it look similar to this:
 #OP	TYPE	DESCRIPTION	CALLOUT INFO	PROGRAM ARG1 ARG2 ARG3 ...
 #======	=======	===============	===============	===============================
 create	id_resolver	uid:*	*		/some/other/program %k %d 600
-create	id_resolver	*	*		/usr/sbin/nfs.idmap %k %d 600
+create	id_resolver	*	*		/usr/sbin/nfsidmap %k %d 600
 
 Notice that the new line was added above the line for the generic program.
 request-key will find the first matching line and corresponding program.  In
 this case, /some/other/program will handle all uid lookups and
-/usr/sbin/nfs.idmap will handle gid, user, and group lookups.
+/usr/sbin/nfsidmap will handle gid, user, and group lookups.
 
 See <file:Documentation/security/keys-request-key.txt> for more information
 about the request-key function.
 
 
 =========
-nfs.idmap
+nfsidmap
 =========
-nfs.idmap is designed to be called by request-key, and should not be run "by
+nfsidmap is designed to be called by request-key, and should not be run "by
 hand".  This program takes two arguments, a serialized key and a key
 description.  The serialized key is first converted into a key_serial_t, and
 then passed as an argument to keyctl_instantiate (both are part of keyutils.h).
 
-The actual lookups are performed by functions found in nfsidmap.h.  nfs.idmap
+The actual lookups are performed by functions found in nfsidmap.h.  nfsidmap
 determines the correct function to call by looking at the first part of the
 description string.  For example, a uid lookup description will appear as
 "uid:user@domain".
 
-nfs.idmap will return 0 if the key was instantiated, and non-zero otherwise.
+nfsidmap will return 0 if the key was instantiated, and non-zero otherwise.
-- 
1.7.6.4

Re: [PATCH 2/2] idmapper.txt: Fix the upcall binary name.

[Bryan Schumaker]

On Thu 10 Nov 2011 03:26:03 PM EST, Steve Dickson wrote:
> The binary that handles the upcalls from is kernel is called
> nfsidmap not nfs.idmap.

I'm surprised this wasn't changed when the binary name changed.  Good 
catch!

- Bryan
>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  Documentation/filesystems/nfs/idmapper.txt |   22 +++++++++++-----------
>  1 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/filesystems/nfs/idmapper.txt b/Documentation/filesystems/nfs/idmapper.txt
> index 120fd3c..9c1925a 100644
> --- a/Documentation/filesystems/nfs/idmapper.txt
> +++ b/Documentation/filesystems/nfs/idmapper.txt
> @@ -6,7 +6,7 @@ Id mapper is used by NFS to translate user and group ids into names, and to
>  translate user and group names into ids.  Part of this translation involves
>  performing an upcall to userspace to request the information.  Id mapper will
>  user request-key to perform this upcall and cache the result.  The program
> -/usr/sbin/nfs.idmap should be called by request-key, and will perform the
> +/usr/sbin/nfsidmap should be called by request-key, and will perform the
>  translation and initialize a key with the resulting information.
>  
>   NFS_USE_NEW_IDMAPPER must be selected when configuring the kernel to use this
> @@ -20,12 +20,12 @@ direct the upcall.  The following line should be added:
>  
>  #OP	TYPE	DESCRIPTION	CALLOUT INFO	PROGRAM ARG1 ARG2 ARG3 ...
>  #======	=======	===============	===============	===============================
> -create	id_resolver	*	*		/usr/sbin/nfs.idmap %k %d 600
> +create	id_resolver	*	*		/usr/sbin/nfsidmap %k %d 600
>  
> -This will direct all id_resolver requests to the program /usr/sbin/nfs.idmap.
> +This will direct all id_resolver requests to the program /usr/sbin/nfsidmap.
>  The last parameter, 600, defines how many seconds into the future the key will
> -expire.  This parameter is optional for /usr/sbin/nfs.idmap.  When the timeout
> -is not specified, nfs.idmap will default to 600 seconds.
> +expire.  This parameter is optional for /usr/sbin/nfsidmap.  When the timeout
> +is not specified, nfsidmap will default to 600 seconds.
>  
>  id mapper uses for key descriptions:
>  	  uid:  Find the UID for the given user
> @@ -40,28 +40,28 @@ would edit your request-key.conf so it look similar to this:
>  #OP	TYPE	DESCRIPTION	CALLOUT INFO	PROGRAM ARG1 ARG2 ARG3 ...
>  #======	=======	===============	===============	===============================
>  create	id_resolver	uid:*	*		/some/other/program %k %d 600
> -create	id_resolver	*	*		/usr/sbin/nfs.idmap %k %d 600
> +create	id_resolver	*	*		/usr/sbin/nfsidmap %k %d 600
>  
>  Notice that the new line was added above the line for the generic program.
>  request-key will find the first matching line and corresponding program.  In
>  this case, /some/other/program will handle all uid lookups and
> -/usr/sbin/nfs.idmap will handle gid, user, and group lookups.
> +/usr/sbin/nfsidmap will handle gid, user, and group lookups.
>  
>  See <file:Documentation/security/keys-request-key.txt> for more information
>  about the request-key function.
>  
>  
>  =========
> -nfs.idmap
> +nfsidmap
>  =========
> -nfs.idmap is designed to be called by request-key, and should not be run "by
> +nfsidmap is designed to be called by request-key, and should not be run "by
>  hand".  This program takes two arguments, a serialized key and a key
>  description.  The serialized key is first converted into a key_serial_t, and
>  then passed as an argument to keyctl_instantiate (both are part of keyutils.h).
>  
> -The actual lookups are performed by functions found in nfsidmap.h.  nfs.idmap
> +The actual lookups are performed by functions found in nfsidmap.h.  nfsidmap
>  determines the correct function to call by looking at the first part of the
>  description string.  For example, a uid lookup description will appear as
>  "uid:user@domain".
>  
> -nfs.idmap will return 0 if the key was instantiated, and non-zero otherwise.
> +nfsidmap will return 0 if the key was instantiated, and non-zero otherwise.

Re: [PATCH 0/2] Enable the in-kernel id mapping to be on by default.

[Trond Myklebust]

On Thu, 2011-11-10 at 15:26 -0500, Steve Dickson wrote: 
> This patch set enables the in-kernel id mapping upcall
> mechanism, which will fix deadlocks that can occur 
> between the rpc.idmapd daemon and the kernel. 

Which deadlocks are you thinking of?

> This set also correct the binary name in the kernel 
> documentation

Note that we should also at some point change the default value of the
nfs4_disable_idmapping module parameter. Setting it to 'Y' so that the
NFS client starts by trying to use numeric u/gids with 'sec=sys' should
be safe, since we automatically fall back to using string-based
owner/group names if the server rejects numeric values.

Also note that older Linux servers return NFS4ERR_BADNAME in cases where
they should be using NFS4ERR_BADOWNER: please can distribution
maintainers make sure that they have applied commit
f6af99ec1b261e21219d5eba99e3af48fc6c32d4 (nfsd4: name->id mapping should
fail with BADOWNER not BADNAME).

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 0/2] Enable the in-kernel id mapping to be on by default.

[Steve Dickson]

On 11/10/2011 03:52 PM, Trond Myklebust wrote:
> On Thu, 2011-11-10 at 15:26 -0500, Steve Dickson wrote: 
>> This patch set enables the in-kernel id mapping upcall
>> mechanism, which will fix deadlocks that can occur 
>> between the rpc.idmapd daemon and the kernel. 
> 
> Which deadlocks are you thinking of?
https://bugzilla.redhat.com/show_bug.cgi?id=730045
https://bugzilla.redhat.com/show_bug.cgi?id=751992
https://bugzilla.redhat.com/show_bug.cgi?id=609252

> 
>> This set also correct the binary name in the kernel 
>> documentation
> 
> Note that we should also at some point change the default value of the
> nfs4_disable_idmapping module parameter. Setting it to 'Y' so that the
> NFS client starts by trying to use numeric u/gids with 'sec=sys' should
> be safe, since we automatically fall back to using string-based
> owner/group names if the server rejects numeric values.
I guess I could look into doing this... 

> 
> Also note that older Linux servers return NFS4ERR_BADNAME in cases where
> they should be using NFS4ERR_BADOWNER: please can distribution
> maintainers make sure that they have applied commit
> f6af99ec1b261e21219d5eba99e3af48fc6c32d4 (nfsd4: name->id mapping should
> fail with BADOWNER not BADNAME).
Dully noted....

steved.