2010-11-30 02:57:50

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 0/4] Allow the admin to turn off NFSv4 uid/gid mapping

The following patches allow the admin to turn off NFSv4 uid/gid mapping
if mounting using AUTH_SYS security.

The new behaviour is enabled using a module parameter,
nfs4_disable_idmapping.

Note that if the server returns an NFS4ERR_BADOWNER error in reply,
then the new behaviour is automatically disabled on that particular
mount point (and the operation that returned the error is retried).

Trond Myklebust (4):
NFSv4: If the server sends us a numeric uid/gid then accept it
NFSv4: Send unmapped uid/gids to the server if the idmapper fails
NFSv4: cleanup idmapper functions to take an nfs_server argument
NFSv4: Send unmapped uid/gids to the server when using auth_sys

fs/nfs/client.c | 16 ++++++++
fs/nfs/idmap.c | 87 +++++++++++++++++++++++++++++++++++---------
fs/nfs/nfs4proc.c | 8 ++++-
fs/nfs/nfs4xdr.c | 18 ++++-----
include/linux/nfs_fs_sb.h | 1 +
include/linux/nfs_idmap.h | 8 ++--
6 files changed, 105 insertions(+), 33 deletions(-)

--
1.7.3.2



2010-11-30 02:57:51

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 3/4] NFSv4: cleanup idmapper functions to take an nfs_server argument

...instead of the nfs_client.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/idmap.c | 24 ++++++++++++------------
fs/nfs/nfs4xdr.c | 18 ++++++++----------
include/linux/nfs_idmap.h | 8 ++++----
3 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 9bf5921..114de76 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -241,21 +241,21 @@ static int nfs_idmap_lookup_id(const char *name, size_t namelen,
return ret;
}

-int nfs_map_name_to_uid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *uid)
+int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_t namelen, __u32 *uid)
{
if (nfs_map_string_to_numeric(name, namelen, uid))
return 0;
return nfs_idmap_lookup_id(name, namelen, "uid", uid);
}

-int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *gid)
+int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size_t namelen, __u32 *gid)
{
if (nfs_map_string_to_numeric(name, namelen, gid))
return 0;
return nfs_idmap_lookup_id(name, namelen, "gid", gid);
}

-int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
+int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
int ret;
ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
@@ -263,7 +263,7 @@ int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buf
ret = nfs_map_numeric_to_string(uid, buf, buflen);
return ret;
}
-int nfs_map_gid_to_group(struct nfs_client *clp, __u32 gid, char *buf, size_t buflen)
+int nfs_map_gid_to_group(const struct nfs_server *server, __u32 gid, char *buf, size_t buflen)
{
int ret;

@@ -729,27 +729,27 @@ static unsigned int fnvhash32(const void *buf, size_t buflen)
return hash;
}

-int nfs_map_name_to_uid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *uid)
+int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_t namelen, __u32 *uid)
{
- struct idmap *idmap = clp->cl_idmap;
+ struct idmap *idmap = server->nfs_client->cl_idmap;

if (nfs_map_string_to_numeric(name, namelen, uid))
return 0;
return nfs_idmap_id(idmap, &idmap->idmap_user_hash, name, namelen, uid);
}

-int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *uid)
+int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size_t namelen, __u32 *uid)
{
- struct idmap *idmap = clp->cl_idmap;
+ struct idmap *idmap = server->nfs_client->cl_idmap;

if (nfs_map_string_to_numeric(name, namelen, uid))
return 0;
return nfs_idmap_id(idmap, &idmap->idmap_group_hash, name, namelen, uid);
}

-int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
+int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
- struct idmap *idmap = clp->cl_idmap;
+ struct idmap *idmap = server->nfs_client->cl_idmap;
int ret;

ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
@@ -757,9 +757,9 @@ int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buf
ret = nfs_map_numeric_to_string(uid, buf, buflen);
return ret;
}
-int nfs_map_gid_to_group(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
+int nfs_map_gid_to_group(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
- struct idmap *idmap = clp->cl_idmap;
+ struct idmap *idmap = server->nfs_client->cl_idmap;
int ret;

ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 9f1826b..0be0277 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -844,7 +844,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap, const
if (iap->ia_valid & ATTR_MODE)
len += 4;
if (iap->ia_valid & ATTR_UID) {
- owner_namelen = nfs_map_uid_to_name(server->nfs_client, iap->ia_uid, owner_name, IDMAP_NAMESZ);
+ owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name, IDMAP_NAMESZ);
if (owner_namelen < 0) {
dprintk("nfs: couldn't resolve uid %d to string\n",
iap->ia_uid);
@@ -856,7 +856,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap, const
len += 4 + (XDR_QUADLEN(owner_namelen) << 2);
}
if (iap->ia_valid & ATTR_GID) {
- owner_grouplen = nfs_map_gid_to_group(server->nfs_client, iap->ia_gid, owner_group, IDMAP_NAMESZ);
+ owner_grouplen = nfs_map_gid_to_group(server, iap->ia_gid, owner_group, IDMAP_NAMESZ);
if (owner_grouplen < 0) {
dprintk("nfs: couldn't resolve gid %d to string\n",
iap->ia_gid);
@@ -3460,7 +3460,7 @@ out_overflow:
}

static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap,
- struct nfs_client *clp, uint32_t *uid, int may_sleep)
+ const struct nfs_server *server, uint32_t *uid, int may_sleep)
{
uint32_t len;
__be32 *p;
@@ -3480,7 +3480,7 @@ static int decode_attr_owner(struct xdr_stream *xdr, uint32_t *bitmap,
if (!may_sleep) {
/* do nothing */
} else if (len < XDR_MAX_NETOBJ) {
- if (nfs_map_name_to_uid(clp, (char *)p, len, uid) == 0)
+ if (nfs_map_name_to_uid(server, (char *)p, len, uid) == 0)
ret = NFS_ATTR_FATTR_OWNER;
else
dprintk("%s: nfs_map_name_to_uid failed!\n",
@@ -3498,7 +3498,7 @@ out_overflow:
}

static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap,
- struct nfs_client *clp, uint32_t *gid, int may_sleep)
+ const struct nfs_server *server, uint32_t *gid, int may_sleep)
{
uint32_t len;
__be32 *p;
@@ -3518,7 +3518,7 @@ static int decode_attr_group(struct xdr_stream *xdr, uint32_t *bitmap,
if (!may_sleep) {
/* do nothing */
} else if (len < XDR_MAX_NETOBJ) {
- if (nfs_map_group_to_gid(clp, (char *)p, len, gid) == 0)
+ if (nfs_map_group_to_gid(server, (char *)p, len, gid) == 0)
ret = NFS_ATTR_FATTR_GROUP;
else
dprintk("%s: nfs_map_group_to_gid failed!\n",
@@ -4017,14 +4017,12 @@ static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t *bitmap,
goto xdr_error;
fattr->valid |= status;

- status = decode_attr_owner(xdr, bitmap, server->nfs_client,
- &fattr->uid, may_sleep);
+ status = decode_attr_owner(xdr, bitmap, server, &fattr->uid, may_sleep);
if (status < 0)
goto xdr_error;
fattr->valid |= status;

- status = decode_attr_group(xdr, bitmap, server->nfs_client,
- &fattr->gid, may_sleep);
+ status = decode_attr_group(xdr, bitmap, server, &fattr->gid, may_sleep);
if (status < 0)
goto xdr_error;
fattr->valid |= status;
diff --git a/include/linux/nfs_idmap.h b/include/linux/nfs_idmap.h
index e8352dc..ee97767 100644
--- a/include/linux/nfs_idmap.h
+++ b/include/linux/nfs_idmap.h
@@ -96,10 +96,10 @@ void nfs_idmap_delete(struct nfs_client *);

#endif /* CONFIG_NFS_USE_NEW_IDMAPPER */

-int nfs_map_name_to_uid(struct nfs_client *, const char *, size_t, __u32 *);
-int nfs_map_group_to_gid(struct nfs_client *, const char *, size_t, __u32 *);
-int nfs_map_uid_to_name(struct nfs_client *, __u32, char *, size_t);
-int nfs_map_gid_to_group(struct nfs_client *, __u32, char *, size_t);
+int nfs_map_name_to_uid(const struct nfs_server *, const char *, size_t, __u32 *);
+int nfs_map_group_to_gid(const struct nfs_server *, const char *, size_t, __u32 *);
+int nfs_map_uid_to_name(const struct nfs_server *, __u32, char *, size_t);
+int nfs_map_gid_to_group(const struct nfs_server *, __u32, char *, size_t);

extern unsigned int nfs_idmap_cache_timeout;
#endif /* __KERNEL__ */
--
1.7.3.2


2010-11-30 02:57:50

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 1/4] NFSv4: If the server sends us a numeric uid/gid then accept it

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/idmap.c | 28 ++++++++++++++++++++++++++--
1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 4e2d9b6..4281da6 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -33,6 +33,24 @@
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/kernel.h>
+
+static int nfs_map_string_to_numeric(const char *name, size_t namelen, __u32 *res)
+{
+ unsigned long val;
+ char buf[16];
+
+ if (memchr(name, '@', namelen) != NULL || namelen >= sizeof(buf))
+ return 0;
+ memcpy(buf, name, namelen);
+ buf[namelen] = '\0';
+ if (strict_strtoul(buf, 0, &val) != 0)
+ return 0;
+ *res = val;
+ return 1;
+}

#ifdef CONFIG_NFS_USE_NEW_IDMAPPER

@@ -42,7 +60,6 @@
#include <linux/keyctl.h>
#include <linux/key-type.h>
#include <linux/rcupdate.h>
-#include <linux/kernel.h>
#include <linux/err.h>

#include <keys/user-type.h>
@@ -221,11 +238,15 @@ static int nfs_idmap_lookup_id(const char *name, size_t namelen,

int nfs_map_name_to_uid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *uid)
{
+ if (nfs_map_string_to_numeric(name, namelen, uid))
+ return 0;
return nfs_idmap_lookup_id(name, namelen, "uid", uid);
}

int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namelen, __u32 *gid)
{
+ if (nfs_map_string_to_numeric(name, namelen, gid))
+ return 0;
return nfs_idmap_lookup_id(name, namelen, "gid", gid);
}

@@ -243,7 +264,6 @@ int nfs_map_gid_to_group(struct nfs_client *clp, __u32 gid, char *buf, size_t bu
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/init.h>
-#include <linux/types.h>
#include <linux/slab.h>
#include <linux/socket.h>
#include <linux/in.h>
@@ -699,6 +719,8 @@ int nfs_map_name_to_uid(struct nfs_client *clp, const char *name, size_t namelen
{
struct idmap *idmap = clp->cl_idmap;

+ if (nfs_map_string_to_numeric(name, namelen, uid))
+ return 0;
return nfs_idmap_id(idmap, &idmap->idmap_user_hash, name, namelen, uid);
}

@@ -706,6 +728,8 @@ int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namele
{
struct idmap *idmap = clp->cl_idmap;

+ if (nfs_map_string_to_numeric(name, namelen, uid))
+ return 0;
return nfs_idmap_id(idmap, &idmap->idmap_group_hash, name, namelen, uid);
}

--
1.7.3.2


2010-11-30 02:57:51

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 2/4] NFSv4: Send unmapped uid/gids to the server if the idmapper fails

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/idmap.c | 30 ++++++++++++++++++++++++++----
1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 4281da6..9bf5921 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -52,6 +52,11 @@ static int nfs_map_string_to_numeric(const char *name, size_t namelen, __u32 *re
return 1;
}

+static int nfs_map_numeric_to_string(__u32 id, char *buf, size_t buflen)
+{
+ return snprintf(buf, buflen, "%u", id);
+}
+
#ifdef CONFIG_NFS_USE_NEW_IDMAPPER

#include <linux/slab.h>
@@ -252,11 +257,20 @@ int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namele

int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
{
- return nfs_idmap_lookup_name(uid, "user", buf, buflen);
+ int ret;
+ ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
+ if (ret < 0)
+ ret = nfs_map_numeric_to_string(uid, buf, buflen);
+ return ret;
}
int nfs_map_gid_to_group(struct nfs_client *clp, __u32 gid, char *buf, size_t buflen)
{
- return nfs_idmap_lookup_name(gid, "group", buf, buflen);
+ int ret;
+
+ ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
+ if (ret < 0)
+ ret = nfs_map_numeric_to_string(gid, buf, buflen);
+ return ret;
}

#else /* CONFIG_NFS_USE_IDMAPPER not defined */
@@ -736,14 +750,22 @@ int nfs_map_group_to_gid(struct nfs_client *clp, const char *name, size_t namele
int nfs_map_uid_to_name(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
{
struct idmap *idmap = clp->cl_idmap;
+ int ret;

- return nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
+ ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
+ if (ret < 0)
+ ret = nfs_map_numeric_to_string(uid, buf, buflen);
+ return ret;
}
int nfs_map_gid_to_group(struct nfs_client *clp, __u32 uid, char *buf, size_t buflen)
{
struct idmap *idmap = clp->cl_idmap;
+ int ret;

- return nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
+ ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
+ if (ret < 0)
+ ret = nfs_map_numeric_to_string(uid, buf, buflen);
+ return ret;
}

#endif /* CONFIG_NFS_USE_NEW_IDMAPPER */
--
1.7.3.2


2010-11-30 16:02:25

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On 11/30/2010 03:17 PM, Trond Myklebust wrote:
> On Tue, 2010-11-30 at 11:44 +0200, Boaz Harrosh wrote:
>> On 11/30/2010 04:57 AM, Trond Myklebust wrote:
>>> Signed-off-by: Trond Myklebust <[email protected]>
>>> ---
>>> fs/nfs/client.c | 16 ++++++++++++++++
>>> fs/nfs/idmap.c | 21 +++++++++++++--------
>>> fs/nfs/nfs4proc.c | 8 +++++++-
>>> include/linux/nfs_fs_sb.h | 1 +
>>> 4 files changed, 37 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>> index 0870d0d..fb84771 100644
>>> --- a/fs/nfs/client.c
>>> +++ b/fs/nfs/client.c
>>> @@ -58,6 +58,11 @@ static LIST_HEAD(nfs_volume_list);
>>> static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
>>>
>>> /*
>>> + * Turn off NFSv4 uid/gid mapping when using AUTH_SYS
>>> + */
>>> +static int nfs4_disable_idmapping = 0;
>>
>> The double negative is a bit hard. I had to read it 3 times to
>> register. Perhaps consider reversing the name and the default
>>
>> +static int nfs4_enable_idmapping = 1;
>
> No. That suggests that idmapping is optional and that you are fine not
> enabling it. What we're doing here is adding in a hack that is not
> tolerated by most servers. People _should_ have to think before
> disabling idmapping.
>

I'm not suggesting anything. I've propose an identical system that
avoids the confusion I had when I first read it in the morning
before my coffee. But if for you default means zero and override/hack
means set to 1, then sure. Just that I never thought that.
Default is 1, set 0 to disable is just as fine for me.

> Cheers
> Trond
>

Thanks
Boaz

2010-11-30 03:15:12

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH 0/4] Allow the admin to turn off NFSv4 uid/gid mapping

Trond Myklebust wrote:

The following patches allow the admin to turn off NFSv4 uid/gid mapping
if mounting using AUTH_SYS security.

The new behaviour is enabled using a module parameter,
nfs4_disable_idmapping.

Would this be more useful as a per-mount option rather than a global?

2010-11-30 02:57:52

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/client.c | 16 ++++++++++++++++
fs/nfs/idmap.c | 21 +++++++++++++--------
fs/nfs/nfs4proc.c | 8 +++++++-
include/linux/nfs_fs_sb.h | 1 +
4 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 0870d0d..fb84771 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -58,6 +58,11 @@ static LIST_HEAD(nfs_volume_list);
static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);

/*
+ * Turn off NFSv4 uid/gid mapping when using AUTH_SYS
+ */
+static int nfs4_disable_idmapping = 0;
+
+/*
* RPC cruft for NFS
*/
static struct rpc_version *nfs_version[5] = {
@@ -1387,6 +1392,13 @@ static int nfs4_init_server(struct nfs_server *server,
if (error < 0)
goto error;

+ /*
+ * Don't use NFS uid/gid mapping if we're using AUTH_SYS or lower
+ * authentication.
+ */
+ if (nfs4_disable_idmapping && data->auth_flavors[0] == RPC_AUTH_UNIX)
+ server->caps |= NFS_CAP_UIDGID_NOMAP;
+
if (data->rsize)
server->rsize = nfs_block_size(data->rsize, NULL);
if (data->wsize)
@@ -1808,3 +1820,7 @@ void nfs_fs_proc_exit(void)
}

#endif /* CONFIG_PROC_FS */
+
+module_param(nfs4_disable_idmapping, bool, 0644);
+MODULE_PARM_DESC(nfs4_disable_idmapping,
+ "Turn off NFSv4 idmapping when using 'sec=sys'");
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 114de76..d816bba 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -257,17 +257,20 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size

int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
- int ret;
- ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
+ int ret = -EINVAL;
+
+ if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
+ ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
if (ret < 0)
ret = nfs_map_numeric_to_string(uid, buf, buflen);
return ret;
}
int nfs_map_gid_to_group(const struct nfs_server *server, __u32 gid, char *buf, size_t buflen)
{
- int ret;
+ int ret = -EINVAL;

- ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
+ if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
+ ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
if (ret < 0)
ret = nfs_map_numeric_to_string(gid, buf, buflen);
return ret;
@@ -750,9 +753,10 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
struct idmap *idmap = server->nfs_client->cl_idmap;
- int ret;
+ int ret = -EINVAL;

- ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
+ if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
+ ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
if (ret < 0)
ret = nfs_map_numeric_to_string(uid, buf, buflen);
return ret;
@@ -760,9 +764,10 @@ int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, s
int nfs_map_gid_to_group(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
{
struct idmap *idmap = server->nfs_client->cl_idmap;
- int ret;
+ int ret = -EINVAL;

- ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
+ if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
+ ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
if (ret < 0)
ret = nfs_map_numeric_to_string(uid, buf, buflen);
return ret;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 89b0430..114547a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -239,7 +239,7 @@ static int nfs4_delay(struct rpc_clnt *clnt, long *timeout)
/* This is the error handling routine for processes that are allowed
* to sleep.
*/
-static int nfs4_handle_exception(const struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
+static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
{
struct nfs_client *clp = server->nfs_client;
struct nfs4_state *state = exception->state;
@@ -290,6 +290,12 @@ static int nfs4_handle_exception(const struct nfs_server *server, int errorcode,
break;
case -NFS4ERR_OLD_STATEID:
exception->retry = 1;
+ break;
+ case -NFS4ERR_BADOWNER:
+ if (server->caps & NFS_CAP_UIDGID_NOMAP) {
+ server->caps &= ~NFS_CAP_UIDGID_NOMAP;
+ exception->retry = 1;
+ }
}
/* We failed to handle the error */
return nfs4_map_errors(ret);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 452d964..6309262 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -177,6 +177,7 @@ struct nfs_server {
#define NFS_CAP_CTIME (1U << 12)
#define NFS_CAP_MTIME (1U << 13)
#define NFS_CAP_POSIX_LOCK (1U << 14)
+#define NFS_CAP_UIDGID_NOMAP (1U << 15)


/* maximum number of slots to use */
--
1.7.3.2


2010-11-30 03:24:02

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 0/4] Allow the admin to turn off NFSv4 uid/gid mapping

On Mon, 2010-11-29 at 22:15 -0500, Jim Rees wrote:
> Trond Myklebust wrote:
>
> The following patches allow the admin to turn off NFSv4 uid/gid mapping
> if mounting using AUTH_SYS security.
>
> The new behaviour is enabled using a module parameter,
> nfs4_disable_idmapping.
>
> Would this be more useful as a per-mount option rather than a global?

Why? The minute the server rejects it, the option is turned off. The
main reasons I can see for wanting to turn it off at mount time is

1) The server already has a different uid/gid mapping set up
2) The server lies in a different NFS domain.

In either one of those two cases, why would you want to use AUTH_SYS in
the first place? It will be broken.

Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2010-11-30 13:17:28

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, 2010-11-30 at 11:44 +0200, Boaz Harrosh wrote:
> On 11/30/2010 04:57 AM, Trond Myklebust wrote:
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> > fs/nfs/client.c | 16 ++++++++++++++++
> > fs/nfs/idmap.c | 21 +++++++++++++--------
> > fs/nfs/nfs4proc.c | 8 +++++++-
> > include/linux/nfs_fs_sb.h | 1 +
> > 4 files changed, 37 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 0870d0d..fb84771 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -58,6 +58,11 @@ static LIST_HEAD(nfs_volume_list);
> > static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
> >
> > /*
> > + * Turn off NFSv4 uid/gid mapping when using AUTH_SYS
> > + */
> > +static int nfs4_disable_idmapping = 0;
>
> The double negative is a bit hard. I had to read it 3 times to
> register. Perhaps consider reversing the name and the default
>
> +static int nfs4_enable_idmapping = 1;

No. That suggests that idmapping is optional and that you are fine not
enabling it. What we're doing here is adding in a hack that is not
tolerated by most servers. People _should_ have to think before
disabling idmapping.

Cheers
Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2010-11-30 09:45:00

by Boaz Harrosh

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On 11/30/2010 04:57 AM, Trond Myklebust wrote:
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/client.c | 16 ++++++++++++++++
> fs/nfs/idmap.c | 21 +++++++++++++--------
> fs/nfs/nfs4proc.c | 8 +++++++-
> include/linux/nfs_fs_sb.h | 1 +
> 4 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 0870d0d..fb84771 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -58,6 +58,11 @@ static LIST_HEAD(nfs_volume_list);
> static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
>
> /*
> + * Turn off NFSv4 uid/gid mapping when using AUTH_SYS
> + */
> +static int nfs4_disable_idmapping = 0;

The double negative is a bit hard. I had to read it 3 times to
register. Perhaps consider reversing the name and the default

+static int nfs4_enable_idmapping = 1;

> +
> +/*
> * RPC cruft for NFS
> */
> static struct rpc_version *nfs_version[5] = {
> @@ -1387,6 +1392,13 @@ static int nfs4_init_server(struct nfs_server *server,
> if (error < 0)
> goto error;
>
> + /*
> + * Don't use NFS uid/gid mapping if we're using AUTH_SYS or lower
> + * authentication.
> + */
> + if (nfs4_disable_idmapping && data->auth_flavors[0] == RPC_AUTH_UNIX)
> + server->caps |= NFS_CAP_UIDGID_NOMAP;
> +
> if (data->rsize)
> server->rsize = nfs_block_size(data->rsize, NULL);
> if (data->wsize)
> @@ -1808,3 +1820,7 @@ void nfs_fs_proc_exit(void)
> }
>
> #endif /* CONFIG_PROC_FS */
> +
> +module_param(nfs4_disable_idmapping, bool, 0644);
> +MODULE_PARM_DESC(nfs4_disable_idmapping,
> + "Turn off NFSv4 idmapping when using 'sec=sys'");
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index 114de76..d816bba 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -257,17 +257,20 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
>
> int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> - int ret;
> - ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
> + int ret = -EINVAL;
> +
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))

Look here and below it is always used as Negative (of negative), perhaps?
+ if (server->caps & NFS_CAP_UIDGID_MAP)


Thanks
Boaz

> + ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> }
> int nfs_map_gid_to_group(const struct nfs_server *server, __u32 gid, char *buf, size_t buflen)
> {
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(gid, buf, buflen);
> return ret;
> @@ -750,9 +753,10 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
> int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> struct idmap *idmap = server->nfs_client->cl_idmap;
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> @@ -760,9 +764,10 @@ int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, s
> int nfs_map_gid_to_group(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> struct idmap *idmap = server->nfs_client->cl_idmap;
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 89b0430..114547a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -239,7 +239,7 @@ static int nfs4_delay(struct rpc_clnt *clnt, long *timeout)
> /* This is the error handling routine for processes that are allowed
> * to sleep.
> */
> -static int nfs4_handle_exception(const struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
> +static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
> {
> struct nfs_client *clp = server->nfs_client;
> struct nfs4_state *state = exception->state;
> @@ -290,6 +290,12 @@ static int nfs4_handle_exception(const struct nfs_server *server, int errorcode,
> break;
> case -NFS4ERR_OLD_STATEID:
> exception->retry = 1;
> + break;
> + case -NFS4ERR_BADOWNER:
> + if (server->caps & NFS_CAP_UIDGID_NOMAP) {
> + server->caps &= ~NFS_CAP_UIDGID_NOMAP;
> + exception->retry = 1;
> + }
> }
> /* We failed to handle the error */
> return nfs4_map_errors(ret);
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 452d964..6309262 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -177,6 +177,7 @@ struct nfs_server {
> #define NFS_CAP_CTIME (1U << 12)
> #define NFS_CAP_MTIME (1U << 13)
> #define NFS_CAP_POSIX_LOCK (1U << 14)
> +#define NFS_CAP_UIDGID_NOMAP (1U << 15)
>
>
> /* maximum number of slots to use */


2011-01-04 21:33:31

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> I finally got around to setting up idmapd properly with libnss-mysql, and
> in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> this patch applied. With this option set on the client, and the server
> set up normally, I get EINVAL from chown's fchowna():
>
> # chown testuser:testuser test
> chown: changing ownership of `test': Invalid argument
> # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> # chown testuser:testuser test
> #
>
> This happened on 2.6.37-rc5-git4, but I just reproduced it with
> 2.6.37-rc8-git5 as well. The server idmapd logs:
>
> rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
>
> (1009999 is the current uid/gid here.)
>
> I think you meant for this to fall back automatically, right?


Did you remember to apply the patch 'NFSv4: Propagate the error
NFS4ERR_BADOWNER to nfs4_do_setattr'?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-01-04 21:59:54

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, 2011-01-04 at 16:57 -0500, Dr. J. Bruce Fields wrote:
> On Tue, Jan 04, 2011 at 04:50:33PM -0500, Trond Myklebust wrote:
> > On Tue, 2011-01-04 at 13:43 -0800, Simon Kirby wrote:
> > > On Tue, Jan 04, 2011 at 04:33:12PM -0500, Trond Myklebust wrote:
> > >
> > > > On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> > > > > I finally got around to setting up idmapd properly with libnss-mysql, and
> > > > > in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> > > > > this patch applied. With this option set on the client, and the server
> > > > > set up normally, I get EINVAL from chown's fchowna():
> > > > >
> > > > > # chown testuser:testuser test
> > > > > chown: changing ownership of `test': Invalid argument
> > > > > # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> > > > > # chown testuser:testuser test
> > > > > #
> > > > >
> > > > > This happened on 2.6.37-rc5-git4, but I just reproduced it with
> > > > > 2.6.37-rc8-git5 as well. The server idmapd logs:
> > > > >
> > > > > rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> > > > > rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> > > > > rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> > > > > rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
> > > > >
> > > > > (1009999 is the current uid/gid here.)
> > > > >
> > > > > I think you meant for this to fall back automatically, right?
> > > >
> > > > Did you remember to apply the patch 'NFSv4: Propagate the error
> > > > NFS4ERR_BADOWNER to nfs4_do_setattr'?
> > >
> > > Yes, that patch is applied as part of the series.
> > >
> > > The -EINVAL is going back to userland's fchownat(). I expected
> > > to see the "Reenabling the idmapper" printk() from within
> > > nfs4_handle_exception(), but this didn't seem to happen.
> >
> > Hmm... Bruce, does the server actually return NFS4ERR_BADOWNER when it
> > is supposed to? As far as I can see, nfs4idmap will consistently return
> > NFS4ERR_BADNAME, which would be a bug here.
>
> Whoops. Looking at the spec.... Looks like BADNAME should be reserved
> only for filenames? I'll fix that now.

Sigh... I'll fix up the client patches to work around the server bug...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-01-04 21:50:35

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, 2011-01-04 at 13:43 -0800, Simon Kirby wrote:
> On Tue, Jan 04, 2011 at 04:33:12PM -0500, Trond Myklebust wrote:
>
> > On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> > > I finally got around to setting up idmapd properly with libnss-mysql, and
> > > in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> > > this patch applied. With this option set on the client, and the server
> > > set up normally, I get EINVAL from chown's fchowna():
> > >
> > > # chown testuser:testuser test
> > > chown: changing ownership of `test': Invalid argument
> > > # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> > > # chown testuser:testuser test
> > > #
> > >
> > > This happened on 2.6.37-rc5-git4, but I just reproduced it with
> > > 2.6.37-rc8-git5 as well. The server idmapd logs:
> > >
> > > rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> > > rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> > > rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> > > rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
> > >
> > > (1009999 is the current uid/gid here.)
> > >
> > > I think you meant for this to fall back automatically, right?
> >
> > Did you remember to apply the patch 'NFSv4: Propagate the error
> > NFS4ERR_BADOWNER to nfs4_do_setattr'?
>
> Yes, that patch is applied as part of the series.
>
> The -EINVAL is going back to userland's fchownat(). I expected
> to see the "Reenabling the idmapper" printk() from within
> nfs4_handle_exception(), but this didn't seem to happen.

Hmm... Bruce, does the server actually return NFS4ERR_BADOWNER when it
is supposed to? As far as I can see, nfs4idmap will consistently return
NFS4ERR_BADNAME, which would be a bug here.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-01-04 23:23:35

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 2/4] nfsd4: move idmap and acl header files into fs/nfsd

These are internal nfsd interfaces.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/acl.h | 61 +++++++++++++++++++++++++++++++++++++++++
fs/nfsd/idmap.h | 64 ++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfs4acl.c | 2 +-
fs/nfsd/nfs4idmap.c | 2 +-
fs/nfsd/nfs4xdr.c | 5 ++-
fs/nfsd/nfsctl.c | 2 +-
fs/nfsd/vfs.c | 4 +-
include/linux/nfs4_acl.h | 61 -----------------------------------------
include/linux/nfsd_idmap.h | 64 --------------------------------------------
9 files changed, 133 insertions(+), 132 deletions(-)
create mode 100644 fs/nfsd/acl.h
create mode 100644 fs/nfsd/idmap.h
delete mode 100644 include/linux/nfs4_acl.h
delete mode 100644 include/linux/nfsd_idmap.h

diff --git a/fs/nfsd/acl.h b/fs/nfsd/acl.h
new file mode 100644
index 0000000..c9c05a7
--- /dev/null
+++ b/fs/nfsd/acl.h
@@ -0,0 +1,61 @@
+/*
+ * include/linux/nfs4_acl.c
+ *
+ * Common NFSv4 ACL handling definitions.
+ *
+ * Copyright (c) 2002 The Regents of the University of Michigan.
+ * All rights reserved.
+ *
+ * Marius Aamodt Eriksen <[email protected]>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef LINUX_NFS4_ACL_H
+#define LINUX_NFS4_ACL_H
+
+#include <linux/posix_acl.h>
+
+/* Maximum ACL we'll accept from client; chosen (somewhat arbitrarily) to
+ * fit in a page: */
+#define NFS4_ACL_MAX 170
+
+struct nfs4_acl *nfs4_acl_new(int);
+int nfs4_acl_get_whotype(char *, u32);
+int nfs4_acl_write_who(int who, char *p);
+int nfs4_acl_permission(struct nfs4_acl *acl, uid_t owner, gid_t group,
+ uid_t who, u32 mask);
+
+#define NFS4_ACL_TYPE_DEFAULT 0x01
+#define NFS4_ACL_DIR 0x02
+#define NFS4_ACL_OWNER 0x04
+
+struct nfs4_acl *nfs4_acl_posix_to_nfsv4(struct posix_acl *,
+ struct posix_acl *, unsigned int flags);
+int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *, struct posix_acl **,
+ struct posix_acl **, unsigned int flags);
+
+#endif /* LINUX_NFS4_ACL_H */
diff --git a/fs/nfsd/idmap.h b/fs/nfsd/idmap.h
new file mode 100644
index 0000000..d4a2ac1
--- /dev/null
+++ b/fs/nfsd/idmap.h
@@ -0,0 +1,64 @@
+/*
+ * include/linux/nfsd_idmap.h
+ *
+ * Mapping of UID to name and vice versa.
+ *
+ * Copyright (c) 2002, 2003 The Regents of the University of
+ * Michigan. All rights reserved.
+> *
+ * Marius Aamodt Eriksen <[email protected]>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef LINUX_NFSD_IDMAP_H
+#define LINUX_NFSD_IDMAP_H
+
+#include <linux/in.h>
+#include <linux/sunrpc/svc.h>
+
+/* XXX from linux/nfs_idmap.h */
+#define IDMAP_NAMESZ 128
+
+#ifdef CONFIG_NFSD_V4
+int nfsd_idmap_init(void);
+void nfsd_idmap_shutdown(void);
+#else
+static inline int nfsd_idmap_init(void)
+{
+ return 0;
+}
+static inline void nfsd_idmap_shutdown(void)
+{
+}
+#endif
+
+int nfsd_map_name_to_uid(struct svc_rqst *, const char *, size_t, __u32 *);
+int nfsd_map_name_to_gid(struct svc_rqst *, const char *, size_t, __u32 *);
+int nfsd_map_uid_to_name(struct svc_rqst *, __u32, char *);
+int nfsd_map_gid_to_name(struct svc_rqst *, __u32, char *);
+
+#endif /* LINUX_NFSD_IDMAP_H */
diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index e480526..ad88f1c 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -36,7 +36,7 @@

#include <linux/slab.h>
#include <linux/nfs_fs.h>
-#include <linux/nfs4_acl.h>
+#include "acl.h"


/* mode bit translations: */
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 844960f..cbd5997 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -33,10 +33,10 @@
*/

#include <linux/module.h>
-#include <linux/nfsd_idmap.h>
#include <linux/seq_file.h>
#include <linux/sched.h>
#include <linux/slab.h>
+#include "idmap.h"

/*
* Cache entry
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 364aae7..2a0814d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -44,13 +44,14 @@
#include <linux/namei.h>
#include <linux/statfs.h>
#include <linux/utsname.h>
-#include <linux/nfsd_idmap.h>
-#include <linux/nfs4_acl.h>
#include <linux/sunrpc/svcauth_gss.h>

+#include "idmap.h"
+#include "acl.h"
#include "xdr4.h"
#include "vfs.h"

+
#define NFSDDBG_FACILITY NFSDDBG_XDR

/*
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 6840ec3..33b3e2b 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -8,12 +8,12 @@
#include <linux/namei.h>
#include <linux/ctype.h>

-#include <linux/nfsd_idmap.h>
#include <linux/sunrpc/svcsock.h>
#include <linux/nfsd/syscall.h>
#include <linux/lockd/lockd.h>
#include <linux/sunrpc/clnt.h>

+#include "idmap.h"
#include "nfsd.h"
#include "cache.h"

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6a3af2f..b991125 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -35,8 +35,8 @@
#endif /* CONFIG_NFSD_V3 */

#ifdef CONFIG_NFSD_V4
-#include <linux/nfs4_acl.h>
-#include <linux/nfsd_idmap.h>
+#include "acl.h"
+#include "idmap.h"
#endif /* CONFIG_NFSD_V4 */

#include "nfsd.h"
diff --git a/include/linux/nfs4_acl.h b/include/linux/nfs4_acl.h
deleted file mode 100644
index c9c05a7..0000000
--- a/include/linux/nfs4_acl.h
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * include/linux/nfs4_acl.c
- *
- * Common NFSv4 ACL handling definitions.
- *
- * Copyright (c) 2002 The Regents of the University of Michigan.
- * All rights reserved.
- *
- * Marius Aamodt Eriksen <[email protected]>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its
- * contributors may be used to endorse or promote products derived
- * from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
- * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef LINUX_NFS4_ACL_H
-#define LINUX_NFS4_ACL_H
-
-#include <linux/posix_acl.h>
-
-/* Maximum ACL we'll accept from client; chosen (somewhat arbitrarily) to
- * fit in a page: */
-#define NFS4_ACL_MAX 170
-
-struct nfs4_acl *nfs4_acl_new(int);
-int nfs4_acl_get_whotype(char *, u32);
-int nfs4_acl_write_who(int who, char *p);
-int nfs4_acl_permission(struct nfs4_acl *acl, uid_t owner, gid_t group,
- uid_t who, u32 mask);
-
-#define NFS4_ACL_TYPE_DEFAULT 0x01
-#define NFS4_ACL_DIR 0x02
-#define NFS4_ACL_OWNER 0x04
-
-struct nfs4_acl *nfs4_acl_posix_to_nfsv4(struct posix_acl *,
- struct posix_acl *, unsigned int flags);
-int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *, struct posix_acl **,
- struct posix_acl **, unsigned int flags);
-
-#endif /* LINUX_NFS4_ACL_H */
diff --git a/include/linux/nfsd_idmap.h b/include/linux/nfsd_idmap.h
deleted file mode 100644
index d4a2ac1..0000000
--- a/include/linux/nfsd_idmap.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * include/linux/nfsd_idmap.h
- *
- * Mapping of UID to name and vice versa.
- *
- * Copyright (c) 2002, 2003 The Regents of the University of
- * Michigan. All rights reserved.
-> *
- * Marius Aamodt Eriksen <[email protected]>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its
- * contributors may be used to endorse or promote products derived
- * from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
- * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef LINUX_NFSD_IDMAP_H
-#define LINUX_NFSD_IDMAP_H
-
-#include <linux/in.h>
-#include <linux/sunrpc/svc.h>
-
-/* XXX from linux/nfs_idmap.h */
-#define IDMAP_NAMESZ 128
-
-#ifdef CONFIG_NFSD_V4
-int nfsd_idmap_init(void);
-void nfsd_idmap_shutdown(void);
-#else
-static inline int nfsd_idmap_init(void)
-{
- return 0;
-}
-static inline void nfsd_idmap_shutdown(void)
-{
-}
-#endif
-
-int nfsd_map_name_to_uid(struct svc_rqst *, const char *, size_t, __u32 *);
-int nfsd_map_name_to_gid(struct svc_rqst *, const char *, size_t, __u32 *);
-int nfsd_map_uid_to_name(struct svc_rqst *, __u32, char *);
-int nfsd_map_gid_to_name(struct svc_rqst *, __u32, char *);
-
-#endif /* LINUX_NFSD_IDMAP_H */
--
1.7.1


2011-01-04 23:23:35

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 4/4] nfsd4: return nfs errno from name_to_id functions

This avoids the need for the confusing ESRCH mapping.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/idmap.h | 4 ++--
fs/nfsd/nfs4idmap.c | 13 +++++++------
fs/nfsd/nfs4xdr.c | 10 +++++-----
fs/nfsd/nfsproc.c | 1 -
4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/idmap.h b/fs/nfsd/idmap.h
index 5147589..2f3be13 100644
--- a/fs/nfsd/idmap.h
+++ b/fs/nfsd/idmap.h
@@ -54,8 +54,8 @@ static inline void nfsd_idmap_shutdown(void)
}
#endif

-int nfsd_map_name_to_uid(struct svc_rqst *, const char *, size_t, __u32 *);
-int nfsd_map_name_to_gid(struct svc_rqst *, const char *, size_t, __u32 *);
+__be32 nfsd_map_name_to_uid(struct svc_rqst *, const char *, size_t, __u32 *);
+__be32 nfsd_map_name_to_gid(struct svc_rqst *, const char *, size_t, __u32 *);
int nfsd_map_uid_to_name(struct svc_rqst *, __u32, char *);
int nfsd_map_gid_to_name(struct svc_rqst *, __u32, char *);

diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index cbd5997..6d2c397 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -37,6 +37,7 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include "idmap.h"
+#include "nfsd.h"

/*
* Cache entry
@@ -514,7 +515,7 @@ rqst_authname(struct svc_rqst *rqstp)
return clp->name;
}

-static int
+static __be32
idmap_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namelen,
uid_t *id)
{
@@ -524,15 +525,15 @@ idmap_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namelen
int ret;

if (namelen + 1 > sizeof(key.name))
- return -ESRCH; /* nfserr_badowner */
+ return nfserr_badowner;
memcpy(key.name, name, namelen);
key.name[namelen] = '\0';
strlcpy(key.authname, rqst_authname(rqstp), sizeof(key.authname));
ret = idmap_lookup(rqstp, nametoid_lookup, &key, &nametoid_cache, &item);
if (ret == -ENOENT)
- ret = -ESRCH; /* nfserr_badowner */
+ return nfserr_badowner;
if (ret)
- return ret;
+ return nfserrno(ret);
*id = item->id;
cache_put(&item->h, &nametoid_cache);
return 0;
@@ -560,14 +561,14 @@ idmap_id_to_name(struct svc_rqst *rqstp, int type, uid_t id, char *name)
return ret;
}

-int
+__be32
nfsd_map_name_to_uid(struct svc_rqst *rqstp, const char *name, size_t namelen,
__u32 *id)
{
return idmap_name_to_id(rqstp, IDMAP_TYPE_USER, name, namelen, id);
}

-int
+__be32
nfsd_map_name_to_gid(struct svc_rqst *rqstp, const char *name, size_t namelen,
__u32 *id)
{
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2a0814d..ca37869 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -289,17 +289,17 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
len += XDR_QUADLEN(dummy32) << 2;
READMEM(buf, dummy32);
ace->whotype = nfs4_acl_get_whotype(buf, dummy32);
- host_err = 0;
+ status = nfs_ok;
if (ace->whotype != NFS4_ACL_WHO_NAMED)
ace->who = 0;
else if (ace->flag & NFS4_ACE_IDENTIFIER_GROUP)
- host_err = nfsd_map_name_to_gid(argp->rqstp,
+ status = nfsd_map_name_to_gid(argp->rqstp,
buf, dummy32, &ace->who);
else
- host_err = nfsd_map_name_to_uid(argp->rqstp,
+ status = nfsd_map_name_to_uid(argp->rqstp,
buf, dummy32, &ace->who);
- if (host_err)
- goto out_nfserr;
+ if (status)
+ return status;
}
} else
*acl = NULL;
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 8f05dcd..e15dc45 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -738,7 +738,6 @@ nfserrno (int errno)
{ nfserr_jukebox, -EAGAIN },
{ nfserr_jukebox, -EWOULDBLOCK },
{ nfserr_jukebox, -ENOMEM },
- { nfserr_badowner, -ESRCH },
{ nfserr_io, -ETXTBSY },
{ nfserr_notsupp, -EOPNOTSUPP },
{ nfserr_toosmall, -ETOOSMALL },
--
1.7.1


2011-01-04 23:18:14

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, Jan 04, 2011 at 04:59:51PM -0500, Trond Myklebust wrote:
> On Tue, 2011-01-04 at 16:57 -0500, Dr. J. Bruce Fields wrote:
> > On Tue, Jan 04, 2011 at 04:50:33PM -0500, Trond Myklebust wrote:
> > > On Tue, 2011-01-04 at 13:43 -0800, Simon Kirby wrote:
> > > > On Tue, Jan 04, 2011 at 04:33:12PM -0500, Trond Myklebust wrote:
> > > >
> > > > > On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> > > > > > I finally got around to setting up idmapd properly with libnss-mysql, and
> > > > > > in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> > > > > > this patch applied. With this option set on the client, and the server
> > > > > > set up normally, I get EINVAL from chown's fchowna():
> > > > > >
> > > > > > # chown testuser:testuser test
> > > > > > chown: changing ownership of `test': Invalid argument
> > > > > > # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> > > > > > # chown testuser:testuser test
> > > > > > #
> > > > > >
> > > > > > This happened on 2.6.37-rc5-git4, but I just reproduced it with
> > > > > > 2.6.37-rc8-git5 as well. The server idmapd logs:
> > > > > >
> > > > > > rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> > > > > > rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> > > > > > rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> > > > > > rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
> > > > > >
> > > > > > (1009999 is the current uid/gid here.)
> > > > > >
> > > > > > I think you meant for this to fall back automatically, right?
> > > > >
> > > > > Did you remember to apply the patch 'NFSv4: Propagate the error
> > > > > NFS4ERR_BADOWNER to nfs4_do_setattr'?
> > > >
> > > > Yes, that patch is applied as part of the series.
> > > >
> > > > The -EINVAL is going back to userland's fchownat(). I expected
> > > > to see the "Reenabling the idmapper" printk() from within
> > > > nfs4_handle_exception(), but this didn't seem to happen.
> > >
> > > Hmm... Bruce, does the server actually return NFS4ERR_BADOWNER when it
> > > is supposed to? As far as I can see, nfs4idmap will consistently return
> > > NFS4ERR_BADNAME, which would be a bug here.
> >
> > Whoops. Looking at the spec.... Looks like BADNAME should be reserved
> > only for filenames? I'll fix that now.
>
> Sigh... I'll fix up the client patches to work around the server bug...

Apologies. Well, it's unambiguous enough, so hopefully it shouldn't
cause problems....

Server fix follows. The first patch is all that matters, the rest is
some cleanup I noticed while I was there.

--b.

2011-01-04 21:25:08

by Simon Kirby

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

I finally got around to setting up idmapd properly with libnss-mysql, and
in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
this patch applied. With this option set on the client, and the server
set up normally, I get EINVAL from chown's fchowna():

# chown testuser:testuser test
chown: changing ownership of `test': Invalid argument
# echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
# chown testuser:testuser test
#

This happened on 2.6.37-rc5-git4, but I just reproduced it with
2.6.37-rc8-git5 as well. The server idmapd logs:

rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"

(1009999 is the current uid/gid here.)

I think you meant for this to fall back automatically, right?

Simon-

On Mon, Nov 29, 2010 at 09:57:43PM -0500, Trond Myklebust wrote:

> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/client.c | 16 ++++++++++++++++
> fs/nfs/idmap.c | 21 +++++++++++++--------
> fs/nfs/nfs4proc.c | 8 +++++++-
> include/linux/nfs_fs_sb.h | 1 +
> 4 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 0870d0d..fb84771 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -58,6 +58,11 @@ static LIST_HEAD(nfs_volume_list);
> static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
>
> /*
> + * Turn off NFSv4 uid/gid mapping when using AUTH_SYS
> + */
> +static int nfs4_disable_idmapping = 0;
> +
> +/*
> * RPC cruft for NFS
> */
> static struct rpc_version *nfs_version[5] = {
> @@ -1387,6 +1392,13 @@ static int nfs4_init_server(struct nfs_server *server,
> if (error < 0)
> goto error;
>
> + /*
> + * Don't use NFS uid/gid mapping if we're using AUTH_SYS or lower
> + * authentication.
> + */
> + if (nfs4_disable_idmapping && data->auth_flavors[0] == RPC_AUTH_UNIX)
> + server->caps |= NFS_CAP_UIDGID_NOMAP;
> +
> if (data->rsize)
> server->rsize = nfs_block_size(data->rsize, NULL);
> if (data->wsize)
> @@ -1808,3 +1820,7 @@ void nfs_fs_proc_exit(void)
> }
>
> #endif /* CONFIG_PROC_FS */
> +
> +module_param(nfs4_disable_idmapping, bool, 0644);
> +MODULE_PARM_DESC(nfs4_disable_idmapping,
> + "Turn off NFSv4 idmapping when using 'sec=sys'");
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index 114de76..d816bba 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -257,17 +257,20 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
>
> int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> - int ret;
> - ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
> + int ret = -EINVAL;
> +
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_lookup_name(uid, "user", buf, buflen);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> }
> int nfs_map_gid_to_group(const struct nfs_server *server, __u32 gid, char *buf, size_t buflen)
> {
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_lookup_name(gid, "group", buf, buflen);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(gid, buf, buflen);
> return ret;
> @@ -750,9 +753,10 @@ int nfs_map_group_to_gid(const struct nfs_server *server, const char *name, size
> int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> struct idmap *idmap = server->nfs_client->cl_idmap;
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> @@ -760,9 +764,10 @@ int nfs_map_uid_to_name(const struct nfs_server *server, __u32 uid, char *buf, s
> int nfs_map_gid_to_group(const struct nfs_server *server, __u32 uid, char *buf, size_t buflen)
> {
> struct idmap *idmap = server->nfs_client->cl_idmap;
> - int ret;
> + int ret = -EINVAL;
>
> - ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
> + if (!(server->caps & NFS_CAP_UIDGID_NOMAP))
> + ret = nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf);
> if (ret < 0)
> ret = nfs_map_numeric_to_string(uid, buf, buflen);
> return ret;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 89b0430..114547a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -239,7 +239,7 @@ static int nfs4_delay(struct rpc_clnt *clnt, long *timeout)
> /* This is the error handling routine for processes that are allowed
> * to sleep.
> */
> -static int nfs4_handle_exception(const struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
> +static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struct nfs4_exception *exception)
> {
> struct nfs_client *clp = server->nfs_client;
> struct nfs4_state *state = exception->state;
> @@ -290,6 +290,12 @@ static int nfs4_handle_exception(const struct nfs_server *server, int errorcode,
> break;
> case -NFS4ERR_OLD_STATEID:
> exception->retry = 1;
> + break;
> + case -NFS4ERR_BADOWNER:
> + if (server->caps & NFS_CAP_UIDGID_NOMAP) {
> + server->caps &= ~NFS_CAP_UIDGID_NOMAP;
> + exception->retry = 1;
> + }
> }
> /* We failed to handle the error */
> return nfs4_map_errors(ret);
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 452d964..6309262 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -177,6 +177,7 @@ struct nfs_server {
> #define NFS_CAP_CTIME (1U << 12)
> #define NFS_CAP_MTIME (1U << 13)
> #define NFS_CAP_POSIX_LOCK (1U << 14)
> +#define NFS_CAP_UIDGID_NOMAP (1U << 15)
>
>
> /* maximum number of slots to use */
> --
> 1.7.3.2
>
> --
> 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

2011-01-04 23:23:35

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 3/4] nfsd4: remove outdated pathname-comments

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/acl.h | 2 --
fs/nfsd/idmap.h | 2 --
2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/acl.h b/fs/nfsd/acl.h
index c9c05a7..34e5c40 100644
--- a/fs/nfsd/acl.h
+++ b/fs/nfsd/acl.h
@@ -1,6 +1,4 @@
/*
- * include/linux/nfs4_acl.c
- *
* Common NFSv4 ACL handling definitions.
*
* Copyright (c) 2002 The Regents of the University of Michigan.
diff --git a/fs/nfsd/idmap.h b/fs/nfsd/idmap.h
index d4a2ac1..5147589 100644
--- a/fs/nfsd/idmap.h
+++ b/fs/nfsd/idmap.h
@@ -1,6 +1,4 @@
/*
- * include/linux/nfsd_idmap.h
- *
* Mapping of UID to name and vice versa.
*
* Copyright (c) 2002, 2003 The Regents of the University of
--
1.7.1


2011-01-04 23:23:35

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 1/4] nfsd4: name->id mapping should fail with BADOWNER not BADNAME

According to rfc 3530 BADNAME is for strings that represent paths;
BADOWNER is for user/group names that don't map.

And the too-long name should probably be BADOWNER as well; it's
effectively the same as if we couldn't map it.

Cc: [email protected]
Reported-by: Trond Myklebust <[email protected]>
Reported-by: Simon Kirby <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4idmap.c | 4 ++--
fs/nfsd/nfsd.h | 1 +
fs/nfsd/nfsproc.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index f0695e8..844960f 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -524,13 +524,13 @@ idmap_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namelen
int ret;

if (namelen + 1 > sizeof(key.name))
- return -EINVAL;
+ return -ESRCH; /* nfserr_badowner */
memcpy(key.name, name, namelen);
key.name[namelen] = '\0';
strlcpy(key.authname, rqst_authname(rqstp), sizeof(key.authname));
ret = idmap_lookup(rqstp, nametoid_lookup, &key, &nametoid_cache, &item);
if (ret == -ENOENT)
- ret = -ESRCH; /* nfserr_badname */
+ ret = -ESRCH; /* nfserr_badowner */
if (ret)
return ret;
*id = item->id;
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 6b641cf..7ecfa24 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -158,6 +158,7 @@ void nfsd_lockd_shutdown(void);
#define nfserr_attrnotsupp cpu_to_be32(NFSERR_ATTRNOTSUPP)
#define nfserr_bad_xdr cpu_to_be32(NFSERR_BAD_XDR)
#define nfserr_openmode cpu_to_be32(NFSERR_OPENMODE)
+#define nfserr_badowner cpu_to_be32(NFSERR_BADOWNER)
#define nfserr_locks_held cpu_to_be32(NFSERR_LOCKS_HELD)
#define nfserr_op_illegal cpu_to_be32(NFSERR_OP_ILLEGAL)
#define nfserr_grace cpu_to_be32(NFSERR_GRACE)
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index fd608a2..8f05dcd 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -738,7 +738,7 @@ nfserrno (int errno)
{ nfserr_jukebox, -EAGAIN },
{ nfserr_jukebox, -EWOULDBLOCK },
{ nfserr_jukebox, -ENOMEM },
- { nfserr_badname, -ESRCH },
+ { nfserr_badowner, -ESRCH },
{ nfserr_io, -ETXTBSY },
{ nfserr_notsupp, -EOPNOTSUPP },
{ nfserr_toosmall, -ETOOSMALL },
--
1.7.1


2011-01-04 21:57:35

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, Jan 04, 2011 at 04:50:33PM -0500, Trond Myklebust wrote:
> On Tue, 2011-01-04 at 13:43 -0800, Simon Kirby wrote:
> > On Tue, Jan 04, 2011 at 04:33:12PM -0500, Trond Myklebust wrote:
> >
> > > On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> > > > I finally got around to setting up idmapd properly with libnss-mysql, and
> > > > in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> > > > this patch applied. With this option set on the client, and the server
> > > > set up normally, I get EINVAL from chown's fchowna():
> > > >
> > > > # chown testuser:testuser test
> > > > chown: changing ownership of `test': Invalid argument
> > > > # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> > > > # chown testuser:testuser test
> > > > #
> > > >
> > > > This happened on 2.6.37-rc5-git4, but I just reproduced it with
> > > > 2.6.37-rc8-git5 as well. The server idmapd logs:
> > > >
> > > > rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> > > > rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> > > > rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> > > > rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
> > > >
> > > > (1009999 is the current uid/gid here.)
> > > >
> > > > I think you meant for this to fall back automatically, right?
> > >
> > > Did you remember to apply the patch 'NFSv4: Propagate the error
> > > NFS4ERR_BADOWNER to nfs4_do_setattr'?
> >
> > Yes, that patch is applied as part of the series.
> >
> > The -EINVAL is going back to userland's fchownat(). I expected
> > to see the "Reenabling the idmapper" printk() from within
> > nfs4_handle_exception(), but this didn't seem to happen.
>
> Hmm... Bruce, does the server actually return NFS4ERR_BADOWNER when it
> is supposed to? As far as I can see, nfs4idmap will consistently return
> NFS4ERR_BADNAME, which would be a bug here.

Whoops. Looking at the spec.... Looks like BADNAME should be reserved
only for filenames? I'll fix that now.

--b.

2011-01-04 21:43:35

by Simon Kirby

[permalink] [raw]
Subject: Re: [PATCH 4/4] NFSv4: Send unmapped uid/gids to the server when using auth_sys

On Tue, Jan 04, 2011 at 04:33:12PM -0500, Trond Myklebust wrote:

> On Tue, 2011-01-04 at 13:25 -0800, Simon Kirby wrote:
> > I finally got around to setting up idmapd properly with libnss-mysql, and
> > in doing so, I forgot that I had enabled nfs4_disable_idmapping=Y with
> > this patch applied. With this option set on the client, and the server
> > set up normally, I get EINVAL from chown's fchowna():
> >
> > # chown testuser:testuser test
> > chown: changing ownership of `test': Invalid argument
> > # echo N > /sys/module/nfs/parameters/nfs4_disable_idmapping
> > # chown testuser:testuser test
> > #
> >
> > This happened on 2.6.37-rc5-git4, but I just reproduced it with
> > 2.6.37-rc8-git5 as well. The server idmapd logs:
> >
> > rpc.idmapd[2987]: nss_getpwnam: name '1009999' does not map into domain 'localdomain'
> > rpc.idmapd[2987]: Server: (user) name "1009999" -> id "65534"
> > rpc.idmapd[2987]: nfsdcb: authbuf=10.10.52.0/24 authtype=group
> > rpc.idmapd[2987]: Server: (group) name "1009999" -> id "65534"
> >
> > (1009999 is the current uid/gid here.)
> >
> > I think you meant for this to fall back automatically, right?
>
> Did you remember to apply the patch 'NFSv4: Propagate the error
> NFS4ERR_BADOWNER to nfs4_do_setattr'?

Yes, that patch is applied as part of the series.

The -EINVAL is going back to userland's fchownat(). I expected
to see the "Reenabling the idmapper" printk() from within
nfs4_handle_exception(), but this didn't seem to happen.

Simon-