[6/9] NFSv4: Convert the NFS client idmapper to use the container user namespace
diff mbox series

Message ID 20190424214650.4658-7-trond.myklebust@hammerspace.com
State New
Headers show
Series
  • Client container fixes
Related show

Commit Message

Trond Myklebust April 24, 2019, 9:46 p.m. UTC
When mapping NFS identities using the NFSv4 idmapper, we want to substitute
for the uids and gids that would normally go on the wire as part of a
NFSv3 request. So we use the same mapping in the NFSv4 upcall as we
use in the NFSv3 RPC call (i.e. the mapping stored in the rpc_clnt cred).

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4idmap.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

Comments

J. Bruce Fields April 25, 2019, 2:32 p.m. UTC | #1
On Wed, Apr 24, 2019 at 05:46:47PM -0400, Trond Myklebust wrote:
> When mapping NFS identities using the NFSv4 idmapper, we want to substitute
> for the uids and gids that would normally go on the wire as part of a
> NFSv3 request. So we use the same mapping in the NFSv4 upcall as we
> use in the NFSv3 RPC call (i.e. the mapping stored in the rpc_clnt cred).

I'm a little lost.  Do I have it right that the following id's are all
the same?:

	- id's used on the wire
	- id's used in NFSv4 idmapper upcalls
	- id's used in the namespace of the mounting process

And is it assumed that those are all in one namespace?  So different
containers can't use different on-the-wire id's?

--b.

> 
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/nfs4idmap.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
> index bf34ddaa2ad7..4884fdae28fb 100644
> --- a/fs/nfs/nfs4idmap.c
> +++ b/fs/nfs/nfs4idmap.c
> @@ -69,8 +69,16 @@ struct idmap {
>  	struct rpc_pipe		*idmap_pipe;
>  	struct idmap_legacy_upcalldata *idmap_upcall_data;
>  	struct mutex		idmap_mutex;
> +	const struct cred	*cred;
>  };
>  
> +static struct user_namespace *idmap_userns(const struct idmap *idmap)
> +{
> +	if (idmap && idmap->cred)
> +		return idmap->cred->user_ns;
> +	return &init_user_ns;
> +}
> +
>  /**
>   * nfs_fattr_init_names - initialise the nfs_fattr owner_name/group_name fields
>   * @fattr: fully initialised struct nfs_fattr
> @@ -271,14 +279,15 @@ static struct key *nfs_idmap_request_key(const char *name, size_t namelen,
>  					 const char *type, struct idmap *idmap)
>  {
>  	char *desc;
> -	struct key *rkey;
> +	struct key *rkey = ERR_PTR(-EAGAIN);
>  	ssize_t ret;
>  
>  	ret = nfs_idmap_get_desc(name, namelen, type, strlen(type), &desc);
>  	if (ret < 0)
>  		return ERR_PTR(ret);
>  
> -	rkey = request_key(&key_type_id_resolver, desc, "");
> +	if (!idmap->cred || idmap->cred->user_ns == &init_user_ns)
> +		rkey = request_key(&key_type_id_resolver, desc, "");
>  	if (IS_ERR(rkey)) {
>  		mutex_lock(&idmap->idmap_mutex);
>  		rkey = request_key_with_auxdata(&key_type_id_resolver_legacy,
> @@ -452,6 +461,9 @@ nfs_idmap_new(struct nfs_client *clp)
>  	if (idmap == NULL)
>  		return -ENOMEM;
>  
> +	mutex_init(&idmap->idmap_mutex);
> +	idmap->cred = get_cred(clp->cl_rpcclient->cl_cred);
> +
>  	rpc_init_pipe_dir_object(&idmap->idmap_pdo,
>  			&nfs_idmap_pipe_dir_object_ops,
>  			idmap);
> @@ -462,7 +474,6 @@ nfs_idmap_new(struct nfs_client *clp)
>  		goto err;
>  	}
>  	idmap->idmap_pipe = pipe;
> -	mutex_init(&idmap->idmap_mutex);
>  
>  	error = rpc_add_pipe_dir_object(clp->cl_net,
>  			&clp->cl_rpcclient->cl_pipedir_objects,
> @@ -475,6 +486,7 @@ nfs_idmap_new(struct nfs_client *clp)
>  err_destroy_pipe:
>  	rpc_destroy_pipe_data(idmap->idmap_pipe);
>  err:
> +	put_cred(idmap->cred);
>  	kfree(idmap);
>  	return error;
>  }
> @@ -491,6 +503,7 @@ nfs_idmap_delete(struct nfs_client *clp)
>  			&clp->cl_rpcclient->cl_pipedir_objects,
>  			&idmap->idmap_pdo);
>  	rpc_destroy_pipe_data(idmap->idmap_pipe);
> +	put_cred(idmap->cred);
>  	kfree(idmap);
>  }
>  
> @@ -735,7 +748,7 @@ int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_
>  	if (!nfs_map_string_to_numeric(name, namelen, &id))
>  		ret = nfs_idmap_lookup_id(name, namelen, "uid", &id, idmap);
>  	if (ret == 0) {
> -		*uid = make_kuid(&init_user_ns, id);
> +		*uid = make_kuid(idmap_userns(idmap), id);
>  		if (!uid_valid(*uid))
>  			ret = -ERANGE;
>  	}
> @@ -752,7 +765,7 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
>  	if (!nfs_map_string_to_numeric(name, namelen, &id))
>  		ret = nfs_idmap_lookup_id(name, namelen, "gid", &id, idmap);
>  	if (ret == 0) {
> -		*gid = make_kgid(&init_user_ns, id);
> +		*gid = make_kgid(idmap_userns(idmap), id);
>  		if (!gid_valid(*gid))
>  			ret = -ERANGE;
>  	}
> @@ -766,7 +779,7 @@ int nfs_map_uid_to_name(const struct nfs_server *server, kuid_t uid, char *buf,
>  	int ret = -EINVAL;
>  	__u32 id;
>  
> -	id = from_kuid(&init_user_ns, uid);
> +	id = from_kuid_munged(idmap_userns(idmap), uid);
>  	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
>  		ret = nfs_idmap_lookup_name(id, "user", buf, buflen, idmap);
>  	if (ret < 0)
> @@ -780,7 +793,7 @@ int nfs_map_gid_to_group(const struct nfs_server *server, kgid_t gid, char *buf,
>  	int ret = -EINVAL;
>  	__u32 id;
>  
> -	id = from_kgid(&init_user_ns, gid);
> +	id = from_kgid_munged(idmap_userns(idmap), gid);
>  	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
>  		ret = nfs_idmap_lookup_name(id, "group", buf, buflen, idmap);
>  	if (ret < 0)
> -- 
> 2.21.0
Trond Myklebust April 25, 2019, 3 p.m. UTC | #2
On Thu, 2019-04-25 at 10:32 -0400, J. Bruce Fields wrote:
> On Wed, Apr 24, 2019 at 05:46:47PM -0400, Trond Myklebust wrote:
> > When mapping NFS identities using the NFSv4 idmapper, we want to
> > substitute
> > for the uids and gids that would normally go on the wire as part of
> > a
> > NFSv3 request. So we use the same mapping in the NFSv4 upcall as we
> > use in the NFSv3 RPC call (i.e. the mapping stored in the rpc_clnt
> > cred).
> 
> I'm a little lost.  Do I have it right that the following id's are
> all
> the same?:
> 
> 	- id's used on the wire
> 	- id's used in NFSv4 idmapper upcalls
> 	- id's used in the namespace of the mounting process
> 
> And is it assumed that those are all in one namespace?  So different
> containers can't use different on-the-wire id's?

The assumption is that if you have enough privileges to mount a
filesystem using the NFS client, then you would also have enough
privileges to run a userspace client, so there is little point in
restricting the NFS client.

So the guiding principle is that a NFS client mount that is started in
a container should behave as if it were started by a process in a "real
VM". That means that the root uid/gid in the container maps to a root
uid/gid on the wire.
Ditto, if there is a need to run the idmapper in the container, then
the expectation is that processes running as 'user' with uid 'u', will
see their creds mapped correctly by the idmapper. Again, that's what
you would see if the process were running in a VM instead of a
container.

Does that all make sense?

So this means that any orchestrator software which may be setting up
NFS mounts as part of setting up the container has 2 options:

   1. Either perform the mount outside the container namespaces, in which
      case the container process uids/gids are mapped from their
      user_namespace into the user_namespace of the orchestrator, and the
      uids/gids on the wire will reflect those mapped uids/gids (so uid 0
      in the container will be mapped to uid xxx where xxx is decided by
      the orchestrator).
   2. Perform the mount inside the container namespaces, in which case the
      container process uids/gids go on the wire as-is.

Cheers
  Trond

> 
> --b.
> 
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> >  fs/nfs/nfs4idmap.c | 27 ++++++++++++++++++++-------
> >  1 file changed, 20 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
> > index bf34ddaa2ad7..4884fdae28fb 100644
> > --- a/fs/nfs/nfs4idmap.c
> > +++ b/fs/nfs/nfs4idmap.c
> > @@ -69,8 +69,16 @@ struct idmap {
> >  	struct rpc_pipe		*idmap_pipe;
> >  	struct idmap_legacy_upcalldata *idmap_upcall_data;
> >  	struct mutex		idmap_mutex;
> > +	const struct cred	*cred;
> >  };
> >  
> > +static struct user_namespace *idmap_userns(const struct idmap
> > *idmap)
> > +{
> > +	if (idmap && idmap->cred)
> > +		return idmap->cred->user_ns;
> > +	return &init_user_ns;
> > +}
> > +
> >  /**
> >   * nfs_fattr_init_names - initialise the nfs_fattr
> > owner_name/group_name fields
> >   * @fattr: fully initialised struct nfs_fattr
> > @@ -271,14 +279,15 @@ static struct key
> > *nfs_idmap_request_key(const char *name, size_t namelen,
> >  					 const char *type, struct idmap
> > *idmap)
> >  {
> >  	char *desc;
> > -	struct key *rkey;
> > +	struct key *rkey = ERR_PTR(-EAGAIN);
> >  	ssize_t ret;
> >  
> >  	ret = nfs_idmap_get_desc(name, namelen, type, strlen(type),
> > &desc);
> >  	if (ret < 0)
> >  		return ERR_PTR(ret);
> >  
> > -	rkey = request_key(&key_type_id_resolver, desc, "");
> > +	if (!idmap->cred || idmap->cred->user_ns == &init_user_ns)
> > +		rkey = request_key(&key_type_id_resolver, desc, "");
> >  	if (IS_ERR(rkey)) {
> >  		mutex_lock(&idmap->idmap_mutex);
> >  		rkey =
> > request_key_with_auxdata(&key_type_id_resolver_legacy,
> > @@ -452,6 +461,9 @@ nfs_idmap_new(struct nfs_client *clp)
> >  	if (idmap == NULL)
> >  		return -ENOMEM;
> >  
> > +	mutex_init(&idmap->idmap_mutex);
> > +	idmap->cred = get_cred(clp->cl_rpcclient->cl_cred);
> > +
> >  	rpc_init_pipe_dir_object(&idmap->idmap_pdo,
> >  			&nfs_idmap_pipe_dir_object_ops,
> >  			idmap);
> > @@ -462,7 +474,6 @@ nfs_idmap_new(struct nfs_client *clp)
> >  		goto err;
> >  	}
> >  	idmap->idmap_pipe = pipe;
> > -	mutex_init(&idmap->idmap_mutex);
> >  
> >  	error = rpc_add_pipe_dir_object(clp->cl_net,
> >  			&clp->cl_rpcclient->cl_pipedir_objects,
> > @@ -475,6 +486,7 @@ nfs_idmap_new(struct nfs_client *clp)
> >  err_destroy_pipe:
> >  	rpc_destroy_pipe_data(idmap->idmap_pipe);
> >  err:
> > +	put_cred(idmap->cred);
> >  	kfree(idmap);
> >  	return error;
> >  }
> > @@ -491,6 +503,7 @@ nfs_idmap_delete(struct nfs_client *clp)
> >  			&clp->cl_rpcclient->cl_pipedir_objects,
> >  			&idmap->idmap_pdo);
> >  	rpc_destroy_pipe_data(idmap->idmap_pipe);
> > +	put_cred(idmap->cred);
> >  	kfree(idmap);
> >  }
> >  
> > @@ -735,7 +748,7 @@ int nfs_map_name_to_uid(const struct nfs_server
> > *server, const char *name, size_
> >  	if (!nfs_map_string_to_numeric(name, namelen, &id))
> >  		ret = nfs_idmap_lookup_id(name, namelen, "uid", &id,
> > idmap);
> >  	if (ret == 0) {
> > -		*uid = make_kuid(&init_user_ns, id);
> > +		*uid = make_kuid(idmap_userns(idmap), id);
> >  		if (!uid_valid(*uid))
> >  			ret = -ERANGE;
> >  	}
> > @@ -752,7 +765,7 @@ int nfs_map_group_to_gid(const struct
> > nfs_server *server, const char *name, size
> >  	if (!nfs_map_string_to_numeric(name, namelen, &id))
> >  		ret = nfs_idmap_lookup_id(name, namelen, "gid", &id,
> > idmap);
> >  	if (ret == 0) {
> > -		*gid = make_kgid(&init_user_ns, id);
> > +		*gid = make_kgid(idmap_userns(idmap), id);
> >  		if (!gid_valid(*gid))
> >  			ret = -ERANGE;
> >  	}
> > @@ -766,7 +779,7 @@ int nfs_map_uid_to_name(const struct nfs_server
> > *server, kuid_t uid, char *buf,
> >  	int ret = -EINVAL;
> >  	__u32 id;
> >  
> > -	id = from_kuid(&init_user_ns, uid);
> > +	id = from_kuid_munged(idmap_userns(idmap), uid);
> >  	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> >  		ret = nfs_idmap_lookup_name(id, "user", buf, buflen,
> > idmap);
> >  	if (ret < 0)
> > @@ -780,7 +793,7 @@ int nfs_map_gid_to_group(const struct
> > nfs_server *server, kgid_t gid, char *buf,
> >  	int ret = -EINVAL;
> >  	__u32 id;
> >  
> > -	id = from_kgid(&init_user_ns, gid);
> > +	id = from_kgid_munged(idmap_userns(idmap), gid);
> >  	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> >  		ret = nfs_idmap_lookup_name(id, "group", buf, buflen,
> > idmap);
> >  	if (ret < 0)
> > -- 
> > 2.21.0
J. Bruce Fields April 25, 2019, 3:33 p.m. UTC | #3
On Thu, Apr 25, 2019 at 03:00:22PM +0000, Trond Myklebust wrote:
> The assumption is that if you have enough privileges to mount a
> filesystem using the NFS client, then you would also have enough
> privileges to run a userspace client, so there is little point in
> restricting the NFS client.
> 
> So the guiding principle is that a NFS client mount that is started in
> a container should behave as if it were started by a process in a "real
> VM". That means that the root uid/gid in the container maps to a root
> uid/gid on the wire.
> Ditto, if there is a need to run the idmapper in the container, then
> the expectation is that processes running as 'user' with uid 'u', will
> see their creds mapped correctly by the idmapper. Again, that's what
> you would see if the process were running in a VM instead of a
> container.
> 
> Does that all make sense?

Yes, thanks!

I thought there was a problem that the idmapper depended on
keyring usermodehelper calls that it was hard to pass namespace
information to.  Did that get fixed and I missed it or or forgot?

> 
> So this means that any orchestrator software which may be setting up
> NFS mounts as part of setting up the container has 2 options:
> 
>    1. Either perform the mount outside the container namespaces, in which
>       case the container process uids/gids are mapped from their
>       user_namespace into the user_namespace of the orchestrator, and the
>       uids/gids on the wire will reflect those mapped uids/gids (so uid 0
>       in the container will be mapped to uid xxx where xxx is decided by
>       the orchestrator).
>    2. Perform the mount inside the container namespaces, in which case the
>       container process uids/gids go on the wire as-is.

OK, great, that sounds perfect.

--b.
Trond Myklebust April 25, 2019, 4:40 p.m. UTC | #4
On Thu, 2019-04-25 at 11:33 -0400, bfields@fieldses.org wrote:
> On Thu, Apr 25, 2019 at 03:00:22PM +0000, Trond Myklebust wrote:
> > The assumption is that if you have enough privileges to mount a
> > filesystem using the NFS client, then you would also have enough
> > privileges to run a userspace client, so there is little point in
> > restricting the NFS client.
> > 
> > So the guiding principle is that a NFS client mount that is started
> > in
> > a container should behave as if it were started by a process in a
> > "real
> > VM". That means that the root uid/gid in the container maps to a
> > root
> > uid/gid on the wire.
> > Ditto, if there is a need to run the idmapper in the container,
> > then
> > the expectation is that processes running as 'user' with uid 'u',
> > will
> > see their creds mapped correctly by the idmapper. Again, that's
> > what
> > you would see if the process were running in a VM instead of a
> > container.
> > 
> > Does that all make sense?
> 
> Yes, thanks!
> 
> I thought there was a problem that the idmapper depended on
> keyring usermodehelper calls that it was hard to pass namespace
> information to.  Did that get fixed and I missed it or or forgot?

No, you are correct, and so we assume the container is using rpc.idmapd
(and rpc.gssd) if it is running NFSv4 with RPCSEC_GSS.

If the keyring upcall gets fixed, then we may allow it to be used in
future kernels.

> > So this means that any orchestrator software which may be setting
> > up
> > NFS mounts as part of setting up the container has 2 options:
> > 
> >    1. Either perform the mount outside the container namespaces, in
> > which
> >       case the container process uids/gids are mapped from their
> >       user_namespace into the user_namespace of the orchestrator,
> > and the
> >       uids/gids on the wire will reflect those mapped uids/gids (so
> > uid 0
> >       in the container will be mapped to uid xxx where xxx is
> > decided by
> >       the orchestrator).
> >    2. Perform the mount inside the container namespaces, in which
> > case the
> >       container process uids/gids go on the wire as-is.
> 
> OK, great, that sounds perfect.
> 
> --b.
J. Bruce Fields April 25, 2019, 4:45 p.m. UTC | #5
On Thu, Apr 25, 2019 at 04:40:18PM +0000, Trond Myklebust wrote:
> On Thu, 2019-04-25 at 11:33 -0400, bfields@fieldses.org wrote:
> > On Thu, Apr 25, 2019 at 03:00:22PM +0000, Trond Myklebust wrote:
> > > The assumption is that if you have enough privileges to mount a
> > > filesystem using the NFS client, then you would also have enough
> > > privileges to run a userspace client, so there is little point in
> > > restricting the NFS client.
> > > 
> > > So the guiding principle is that a NFS client mount that is started
> > > in
> > > a container should behave as if it were started by a process in a
> > > "real
> > > VM". That means that the root uid/gid in the container maps to a
> > > root
> > > uid/gid on the wire.
> > > Ditto, if there is a need to run the idmapper in the container,
> > > then
> > > the expectation is that processes running as 'user' with uid 'u',
> > > will
> > > see their creds mapped correctly by the idmapper. Again, that's
> > > what
> > > you would see if the process were running in a VM instead of a
> > > container.
> > > 
> > > Does that all make sense?
> > 
> > Yes, thanks!
> > 
> > I thought there was a problem that the idmapper depended on
> > keyring usermodehelper calls that it was hard to pass namespace
> > information to.  Did that get fixed and I missed it or or forgot?
> 
> No, you are correct, and so we assume the container is using rpc.idmapd
> (and rpc.gssd) if it is running NFSv4 with RPCSEC_GSS.
> 
> If the keyring upcall gets fixed, then we may allow it to be used in
> future kernels.

OK, got it.  Is there anything we lose by using idmapd?  (IDMAP_NAMESZ
might be a limitation?)

--b.
Trond Myklebust April 25, 2019, 4:48 p.m. UTC | #6
On Thu, 2019-04-25 at 12:45 -0400, bfields@fieldses.org wrote:
> On Thu, Apr 25, 2019 at 04:40:18PM +0000, Trond Myklebust wrote:
> > On Thu, 2019-04-25 at 11:33 -0400, bfields@fieldses.org wrote:
> > > On Thu, Apr 25, 2019 at 03:00:22PM +0000, Trond Myklebust wrote:
> > > > The assumption is that if you have enough privileges to mount a
> > > > filesystem using the NFS client, then you would also have
> > > > enough
> > > > privileges to run a userspace client, so there is little point
> > > > in
> > > > restricting the NFS client.
> > > > 
> > > > So the guiding principle is that a NFS client mount that is
> > > > started
> > > > in
> > > > a container should behave as if it were started by a process in
> > > > a
> > > > "real
> > > > VM". That means that the root uid/gid in the container maps to
> > > > a
> > > > root
> > > > uid/gid on the wire.
> > > > Ditto, if there is a need to run the idmapper in the container,
> > > > then
> > > > the expectation is that processes running as 'user' with uid
> > > > 'u',
> > > > will
> > > > see their creds mapped correctly by the idmapper. Again, that's
> > > > what
> > > > you would see if the process were running in a VM instead of a
> > > > container.
> > > > 
> > > > Does that all make sense?
> > > 
> > > Yes, thanks!
> > > 
> > > I thought there was a problem that the idmapper depended on
> > > keyring usermodehelper calls that it was hard to pass namespace
> > > information to.  Did that get fixed and I missed it or or forgot?
> > 
> > No, you are correct, and so we assume the container is using
> > rpc.idmapd
> > (and rpc.gssd) if it is running NFSv4 with RPCSEC_GSS.
> > 
> > If the keyring upcall gets fixed, then we may allow it to be used
> > in
> > future kernels.
> 
> OK, got it.  Is there anything we lose by using
> idmapd?  (IDMAP_NAMESZ
> might be a limitation?)

IDMAP_NAMESZ should be set to 128 by default, so should work for most
cases. I don't think there are any further limitations.
J. Bruce Fields April 25, 2019, 8:16 p.m. UTC | #7
On Thu, Apr 25, 2019 at 04:48:54PM +0000, Trond Myklebust wrote:
> On Thu, 2019-04-25 at 12:45 -0400, bfields@fieldses.org wrote:
> > On Thu, Apr 25, 2019 at 04:40:18PM +0000, Trond Myklebust wrote:
> > > On Thu, 2019-04-25 at 11:33 -0400, bfields@fieldses.org wrote:
> > > > On Thu, Apr 25, 2019 at 03:00:22PM +0000, Trond Myklebust wrote:
> > > > > The assumption is that if you have enough privileges to mount a
> > > > > filesystem using the NFS client, then you would also have
> > > > > enough
> > > > > privileges to run a userspace client, so there is little point
> > > > > in
> > > > > restricting the NFS client.
> > > > > 
> > > > > So the guiding principle is that a NFS client mount that is
> > > > > started
> > > > > in
> > > > > a container should behave as if it were started by a process in
> > > > > a
> > > > > "real
> > > > > VM". That means that the root uid/gid in the container maps to
> > > > > a
> > > > > root
> > > > > uid/gid on the wire.
> > > > > Ditto, if there is a need to run the idmapper in the container,
> > > > > then
> > > > > the expectation is that processes running as 'user' with uid
> > > > > 'u',
> > > > > will
> > > > > see their creds mapped correctly by the idmapper. Again, that's
> > > > > what
> > > > > you would see if the process were running in a VM instead of a
> > > > > container.
> > > > > 
> > > > > Does that all make sense?
> > > > 
> > > > Yes, thanks!
> > > > 
> > > > I thought there was a problem that the idmapper depended on
> > > > keyring usermodehelper calls that it was hard to pass namespace
> > > > information to.  Did that get fixed and I missed it or or forgot?
> > > 
> > > No, you are correct, and so we assume the container is using
> > > rpc.idmapd
> > > (and rpc.gssd) if it is running NFSv4 with RPCSEC_GSS.
> > > 
> > > If the keyring upcall gets fixed, then we may allow it to be used
> > > in
> > > future kernels.
> > 
> > OK, got it.  Is there anything we lose by using
> > idmapd?  (IDMAP_NAMESZ
> > might be a limitation?)
> 
> IDMAP_NAMESZ should be set to 128 by default, so should work for most
> cases. I don't think there are any further limitations.

Makes sense.  Looks good to me!

--b.

Patch
diff mbox series

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index bf34ddaa2ad7..4884fdae28fb 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -69,8 +69,16 @@  struct idmap {
 	struct rpc_pipe		*idmap_pipe;
 	struct idmap_legacy_upcalldata *idmap_upcall_data;
 	struct mutex		idmap_mutex;
+	const struct cred	*cred;
 };
 
+static struct user_namespace *idmap_userns(const struct idmap *idmap)
+{
+	if (idmap && idmap->cred)
+		return idmap->cred->user_ns;
+	return &init_user_ns;
+}
+
 /**
  * nfs_fattr_init_names - initialise the nfs_fattr owner_name/group_name fields
  * @fattr: fully initialised struct nfs_fattr
@@ -271,14 +279,15 @@  static struct key *nfs_idmap_request_key(const char *name, size_t namelen,
 					 const char *type, struct idmap *idmap)
 {
 	char *desc;
-	struct key *rkey;
+	struct key *rkey = ERR_PTR(-EAGAIN);
 	ssize_t ret;
 
 	ret = nfs_idmap_get_desc(name, namelen, type, strlen(type), &desc);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	rkey = request_key(&key_type_id_resolver, desc, "");
+	if (!idmap->cred || idmap->cred->user_ns == &init_user_ns)
+		rkey = request_key(&key_type_id_resolver, desc, "");
 	if (IS_ERR(rkey)) {
 		mutex_lock(&idmap->idmap_mutex);
 		rkey = request_key_with_auxdata(&key_type_id_resolver_legacy,
@@ -452,6 +461,9 @@  nfs_idmap_new(struct nfs_client *clp)
 	if (idmap == NULL)
 		return -ENOMEM;
 
+	mutex_init(&idmap->idmap_mutex);
+	idmap->cred = get_cred(clp->cl_rpcclient->cl_cred);
+
 	rpc_init_pipe_dir_object(&idmap->idmap_pdo,
 			&nfs_idmap_pipe_dir_object_ops,
 			idmap);
@@ -462,7 +474,6 @@  nfs_idmap_new(struct nfs_client *clp)
 		goto err;
 	}
 	idmap->idmap_pipe = pipe;
-	mutex_init(&idmap->idmap_mutex);
 
 	error = rpc_add_pipe_dir_object(clp->cl_net,
 			&clp->cl_rpcclient->cl_pipedir_objects,
@@ -475,6 +486,7 @@  nfs_idmap_new(struct nfs_client *clp)
 err_destroy_pipe:
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
 err:
+	put_cred(idmap->cred);
 	kfree(idmap);
 	return error;
 }
@@ -491,6 +503,7 @@  nfs_idmap_delete(struct nfs_client *clp)
 			&clp->cl_rpcclient->cl_pipedir_objects,
 			&idmap->idmap_pdo);
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
+	put_cred(idmap->cred);
 	kfree(idmap);
 }
 
@@ -735,7 +748,7 @@  int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_
 	if (!nfs_map_string_to_numeric(name, namelen, &id))
 		ret = nfs_idmap_lookup_id(name, namelen, "uid", &id, idmap);
 	if (ret == 0) {
-		*uid = make_kuid(&init_user_ns, id);
+		*uid = make_kuid(idmap_userns(idmap), id);
 		if (!uid_valid(*uid))
 			ret = -ERANGE;
 	}
@@ -752,7 +765,7 @@  int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
 	if (!nfs_map_string_to_numeric(name, namelen, &id))
 		ret = nfs_idmap_lookup_id(name, namelen, "gid", &id, idmap);
 	if (ret == 0) {
-		*gid = make_kgid(&init_user_ns, id);
+		*gid = make_kgid(idmap_userns(idmap), id);
 		if (!gid_valid(*gid))
 			ret = -ERANGE;
 	}
@@ -766,7 +779,7 @@  int nfs_map_uid_to_name(const struct nfs_server *server, kuid_t uid, char *buf,
 	int ret = -EINVAL;
 	__u32 id;
 
-	id = from_kuid(&init_user_ns, uid);
+	id = from_kuid_munged(idmap_userns(idmap), uid);
 	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
 		ret = nfs_idmap_lookup_name(id, "user", buf, buflen, idmap);
 	if (ret < 0)
@@ -780,7 +793,7 @@  int nfs_map_gid_to_group(const struct nfs_server *server, kgid_t gid, char *buf,
 	int ret = -EINVAL;
 	__u32 id;
 
-	id = from_kgid(&init_user_ns, gid);
+	id = from_kgid_munged(idmap_userns(idmap), gid);
 	if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
 		ret = nfs_idmap_lookup_name(id, "group", buf, buflen, idmap);
 	if (ret < 0)