From: Trond Myklebust NFSv4: Bugfixes and cleanups for the NFSv4 client name to uid mapper. Includes a fix by Tim Woods to deal with a caching bug in the case where a user and a group share the same numerical id and/or name. --- fs/nfs/idmap.c | 372 ++++++++++++++++++++++++---------------------- fs/nfs/nfs4xdr.c | 32 +-- include/linux/nfs_idmap.h | 17 +- 3 files changed, 219 insertions(+), 202 deletions(-) diff -puN fs/nfs/idmap.c~nfs-06-fix_idmap2 fs/nfs/idmap.c --- 25/fs/nfs/idmap.c~nfs-06-fix_idmap2 2004-01-09 22:16:12.000000000 -0800 +++ 25-akpm/fs/nfs/idmap.c 2004-01-09 22:16:12.000000000 -0800 @@ -52,14 +52,16 @@ #include #define IDMAP_HASH_SZ 128 -#define IDMAP_HASH_TYPE_NAME 0x01 -#define IDMAP_HASH_TYPE_ID 0x02 -#define IDMAP_HASH_TYPE_INSERT 0x04 struct idmap_hashent { - uid_t ih_id; - char ih_name[IDMAP_NAMESZ]; - u_int32_t ih_namelen; + __u32 ih_id; + int ih_namelen; + char ih_name[IDMAP_NAMESZ]; +}; + +struct idmap_hashtable { + __u8 h_type; + struct idmap_hashent h_entries[IDMAP_HASH_SZ]; }; struct idmap { @@ -67,12 +69,10 @@ struct idmap { struct dentry *idmap_dentry; wait_queue_head_t idmap_wq; struct idmap_msg idmap_im; - struct nfs_server *idmap_server; - struct semaphore idmap_lock; - struct semaphore idmap_im_lock; - struct semaphore idmap_hash_lock; - struct idmap_hashent idmap_id_hash[IDMAP_HASH_SZ]; - struct idmap_hashent idmap_name_hash[IDMAP_HASH_SZ]; + struct semaphore idmap_lock; /* Serializes upcalls */ + struct semaphore idmap_im_lock; /* Protects the hashtable */ + struct idmap_hashtable idmap_user_hash; + struct idmap_hashtable idmap_group_hash; }; static ssize_t idmap_pipe_upcall(struct file *, struct rpc_pipe_msg *, char *, @@ -80,10 +80,7 @@ static ssize_t idmap_pipe_upcall(struc static ssize_t idmap_pipe_downcall(struct file *, const char *, size_t); void idmap_pipe_destroy_msg(struct rpc_pipe_msg *); -static int validate_ascii(char *, u_int32_t); - -static u_int32_t fnvhash32(void *, u_int32_t); -static int idmap_cache_lookup(struct idmap *, int, char *, u_int32_t *, uid_t *); +static unsigned int fnvhash32(const void *, size_t); static struct rpc_pipe_ops idmap_upcall_ops = { .upcall = idmap_pipe_upcall, @@ -101,20 +98,19 @@ nfs_idmap_new(struct nfs_server *server) memset(idmap, 0, sizeof(*idmap)); - idmap->idmap_server = server; - snprintf(idmap->idmap_path, sizeof(idmap->idmap_path), - "%s/idmap", idmap->idmap_server->client->cl_pathname); + "%s/idmap", server->client->cl_pathname); idmap->idmap_dentry = rpc_mkpipe(idmap->idmap_path, - idmap->idmap_server, &idmap_upcall_ops, 0); + idmap, &idmap_upcall_ops, 0); if (IS_ERR(idmap->idmap_dentry)) goto err_free; init_MUTEX(&idmap->idmap_lock); init_MUTEX(&idmap->idmap_im_lock); - init_MUTEX(&idmap->idmap_hash_lock); init_waitqueue_head(&idmap->idmap_wq); + idmap->idmap_user_hash.h_type = IDMAP_TYPE_USER; + idmap->idmap_group_hash.h_type = IDMAP_TYPE_GROUP; return (idmap); @@ -136,34 +132,101 @@ nfs_idmap_delete(struct nfs_server *serv } /* + * Helper routines for manipulating the hashtable + */ +static inline struct idmap_hashent * +idmap_name_hash(struct idmap_hashtable* h, const char *name, size_t len) +{ + return &h->h_entries[fnvhash32(name, len) % IDMAP_HASH_SZ]; +} + +static struct idmap_hashent * +idmap_lookup_name(struct idmap_hashtable *h, const char *name, size_t len) +{ + struct idmap_hashent *he = idmap_name_hash(h, name, len); + + if (he->ih_namelen != len || memcmp(he->ih_name, name, len) != 0) + return NULL; + return he; +} + +static inline struct idmap_hashent * +idmap_id_hash(struct idmap_hashtable* h, __u32 id) +{ + return &h->h_entries[fnvhash32(&id, sizeof(id)) % IDMAP_HASH_SZ]; +} + +static struct idmap_hashent * +idmap_lookup_id(struct idmap_hashtable *h, __u32 id) +{ + struct idmap_hashent *he = idmap_id_hash(h, id); + if (he->ih_id != id || he->ih_namelen == 0) + return NULL; + return he; +} + +/* + * Routines for allocating new entries in the hashtable. + * For now, we just have 1 entry per bucket, so it's all + * pretty trivial. + */ +static inline struct idmap_hashent * +idmap_alloc_name(struct idmap_hashtable *h, char *name, unsigned len) +{ + return idmap_name_hash(h, name, len); +} + +static inline struct idmap_hashent * +idmap_alloc_id(struct idmap_hashtable *h, __u32 id) +{ + return idmap_id_hash(h, id); +} + +static void +idmap_update_entry(struct idmap_hashent *he, const char *name, + size_t namelen, __u32 id) +{ + he->ih_id = id; + memcpy(he->ih_name, name, namelen); + he->ih_name[namelen] = '\0'; + he->ih_namelen = namelen; +} + +/* * Name -> ID */ -int -nfs_idmap_id(struct nfs_server *server, u_int8_t type, char *name, - u_int namelen, uid_t *id) +static int +nfs_idmap_id(struct idmap *idmap, struct idmap_hashtable *h, + const char *name, size_t namelen, __u32 *id) { struct rpc_pipe_msg msg; - struct idmap *idmap = server->idmap; struct idmap_msg *im; + struct idmap_hashent *he; DECLARE_WAITQUEUE(wq, current); - int ret = -1, hashtype = IDMAP_HASH_TYPE_NAME; - u_int xnamelen = namelen; - - if (idmap == NULL) - return (-1); + int ret = -EIO; im = &idmap->idmap_im; - if (namelen > IDMAP_NAMESZ || namelen == 0) - return (-1); + /* + * String sanity checks + * Note that the userland daemon expects NUL terminated strings + */ + for (;;) { + if (namelen == 0) + return -EINVAL; + if (name[namelen-1] != '\0') + break; + namelen--; + } + if (namelen >= IDMAP_NAMESZ) + return -EINVAL; down(&idmap->idmap_lock); down(&idmap->idmap_im_lock); - if (name[xnamelen - 1] == '\0') - xnamelen--; - - if (idmap_cache_lookup(idmap, hashtype, name, &xnamelen, id) == 0) { + he = idmap_lookup_name(h, name, namelen); + if (he != NULL) { + *id = he->ih_id; ret = 0; goto out; } @@ -171,7 +234,7 @@ nfs_idmap_id(struct nfs_server *server, memset(im, 0, sizeof(*im)); memcpy(im->im_name, name, namelen); - im->im_type = type; + im->im_type = h->h_type; im->im_conv = IDMAP_CONV_NAMETOID; memset(&msg, 0, sizeof(msg)); @@ -191,16 +254,9 @@ nfs_idmap_id(struct nfs_server *server, remove_wait_queue(&idmap->idmap_wq, &wq); down(&idmap->idmap_im_lock); - /* - * XXX Race condition here, with testing for status. Go ahead - * and and do the cace lookup anyway. - */ if (im->im_status & IDMAP_STATUS_SUCCESS) { - ret = 0; *id = im->im_id; - - hashtype |= IDMAP_HASH_TYPE_INSERT; - ret = idmap_cache_lookup(idmap, hashtype, name, &xnamelen, id); + ret = 0; } out: @@ -213,35 +269,31 @@ nfs_idmap_id(struct nfs_server *server, /* * ID -> Name */ -int -nfs_idmap_name(struct nfs_server *server, u_int8_t type, uid_t id, - char *name, u_int *namelen) +static int +nfs_idmap_name(struct idmap *idmap, struct idmap_hashtable *h, + __u32 id, char *name) { struct rpc_pipe_msg msg; - struct idmap *idmap = server->idmap; struct idmap_msg *im; + struct idmap_hashent *he; DECLARE_WAITQUEUE(wq, current); - int ret = -1, hashtype = IDMAP_HASH_TYPE_ID; - u_int len; - - if (idmap == NULL) - return (-1); + int ret = -EIO; + unsigned int len; im = &idmap->idmap_im; - if (*namelen < IDMAP_NAMESZ || *namelen == 0) - return (-1); - down(&idmap->idmap_lock); down(&idmap->idmap_im_lock); - if (idmap_cache_lookup(idmap, hashtype, name, namelen, &id) == 0) { - ret = 0; + he = idmap_lookup_id(h, id); + if (he != 0) { + memcpy(name, he->ih_name, he->ih_namelen); + ret = he->ih_namelen; goto out; } memset(im, 0, sizeof(*im)); - im->im_type = type; + im->im_type = h->h_type; im->im_conv = IDMAP_CONV_IDTONAME; im->im_id = id; @@ -256,9 +308,6 @@ nfs_idmap_name(struct nfs_server *server goto out; } - /* - * XXX add timeouts here - */ set_current_state(TASK_UNINTERRUPTIBLE); up(&idmap->idmap_im_lock); schedule(); @@ -267,23 +316,20 @@ nfs_idmap_name(struct nfs_server *server down(&idmap->idmap_im_lock); if (im->im_status & IDMAP_STATUS_SUCCESS) { - if ((len = validate_ascii(im->im_name, IDMAP_NAMESZ)) == -1) + if ((len = strnlen(im->im_name, IDMAP_NAMESZ)) == 0) goto out; - ret = 0; memcpy(name, im->im_name, len); - *namelen = len; - - hashtype |= IDMAP_HASH_TYPE_INSERT; - ret = idmap_cache_lookup(idmap, hashtype, name, namelen, &id); + ret = len; } out: memset(im, 0, sizeof(*im)); up(&idmap->idmap_im_lock); up(&idmap->idmap_lock); - return (ret); + return ret; } +/* RPC pipefs upcall/downcall routines */ static ssize_t idmap_pipe_upcall(struct file *filp, struct rpc_pipe_msg *msg, char *dst, size_t buflen) @@ -310,10 +356,12 @@ static ssize_t idmap_pipe_downcall(struct file *filp, const char *src, size_t mlen) { struct rpc_inode *rpci = RPC_I(filp->f_dentry->d_inode); - struct nfs_server *server = rpci->private; - struct idmap *idmap = server->idmap; + struct idmap *idmap = (struct idmap *)rpci->private; struct idmap_msg im_in, *im = &idmap->idmap_im; - int match = 0, hashtype, badmsg = 0, namelen_in, namelen; + struct idmap_hashtable *h; + struct idmap_hashent *he = NULL; + int namelen_in; + int ret; if (mlen != sizeof(im_in)) return (-ENOSPC); @@ -323,39 +371,66 @@ idmap_pipe_downcall(struct file *filp, c down(&idmap->idmap_im_lock); - namelen_in = validate_ascii(im_in.im_name, IDMAP_NAMESZ); - namelen = validate_ascii(im->im_name, IDMAP_NAMESZ); + ret = mlen; + im->im_status = im_in.im_status; + /* If we got an error, terminate now, and wake up pending upcalls */ + if (!(im_in.im_status & IDMAP_STATUS_SUCCESS)) { + wake_up(&idmap->idmap_wq); + goto out; + } - badmsg = !(im_in.im_status & IDMAP_STATUS_SUCCESS) || namelen_in <= 0; + /* Sanity checking of strings */ + ret = -EINVAL; + namelen_in = strnlen(im_in.im_name, IDMAP_NAMESZ); + if (namelen_in == 0 || namelen_in == IDMAP_NAMESZ) + goto out; + + switch (im_in.im_type) { + case IDMAP_TYPE_USER: + h = &idmap->idmap_user_hash; + break; + case IDMAP_TYPE_GROUP: + h = &idmap->idmap_group_hash; + break; + default: + goto out; + } switch (im_in.im_conv) { case IDMAP_CONV_IDTONAME: - match = im->im_id == im_in.im_id; + /* Did we match the current upcall? */ + if (im->im_conv == IDMAP_CONV_IDTONAME + && im->im_type == im_in.im_type + && im->im_id == im_in.im_id) { + /* Yes: copy string, including the terminating '\0' */ + memcpy(im->im_name, im_in.im_name, namelen_in); + im->im_name[namelen_in] = '\0'; + wake_up(&idmap->idmap_wq); + } + he = idmap_alloc_id(h, im_in.im_id); break; case IDMAP_CONV_NAMETOID: - match = namelen == namelen_in && - memcmp(im->im_name, im_in.im_name, namelen) == 0; + /* Did we match the current upcall? */ + if (im->im_conv == IDMAP_CONV_NAMETOID + && im->im_type == im_in.im_type + && strnlen(im->im_name, IDMAP_NAMESZ) == namelen_in + && memcmp(im->im_name, im_in.im_name, namelen_in) == 0) { + im->im_id = im_in.im_id; + wake_up(&idmap->idmap_wq); + } + he = idmap_alloc_name(h, im_in.im_name, namelen_in); break; default: - badmsg = 1; - break; - } - - match = match && im->im_type == im_in.im_type; - - if (match) { - memcpy(im, &im_in, sizeof(*im)); - wake_up(&idmap->idmap_wq); - } else if (!badmsg) { - hashtype = im_in.im_conv == IDMAP_CONV_IDTONAME ? - IDMAP_HASH_TYPE_ID : IDMAP_HASH_TYPE_NAME; - hashtype |= IDMAP_HASH_TYPE_INSERT; - idmap_cache_lookup(idmap, hashtype, im_in.im_name, &namelen_in, - &im_in.im_id); + goto out; } + /* If the entry is valid, also copy it to the cache */ + if (he != NULL) + idmap_update_entry(he, im_in.im_name, namelen_in, im_in.im_id); + ret = mlen; +out: up(&idmap->idmap_im_lock); - return (mlen); + return ret; } void @@ -372,108 +447,51 @@ idmap_pipe_destroy_msg(struct rpc_pipe_m up(&idmap->idmap_im_lock); } -static int -validate_ascii(char *string, u_int32_t len) -{ - int i; - - for (i = 0; i < len; i++) { - if (string[i] == '\0') - break; - - if (string[i] & 0x80) - return (-1); - } - - if (string[i] != '\0') - return (-1); - - return (i); -} - /* * Fowler/Noll/Vo hash * http://www.isthe.com/chongo/tech/comp/fnv/ */ -#define FNV_P_32 ((u_int32_t)0x01000193) /* 16777619 */ -#define FNV_1_32 ((u_int32_t)0x811c9dc5) /* 2166136261 */ +#define FNV_P_32 ((unsigned int)0x01000193) /* 16777619 */ +#define FNV_1_32 ((unsigned int)0x811c9dc5) /* 2166136261 */ -static u_int32_t -fnvhash32(void *buf, u_int32_t buflen) +static unsigned int fnvhash32(const void *buf, size_t buflen) { - u_char *p, *end = (u_char *)buf + buflen; - u_int32_t hash = FNV_1_32; + const unsigned char *p, *end = (const unsigned char *)buf + buflen; + unsigned int hash = FNV_1_32; for (p = buf; p < end; p++) { hash *= FNV_P_32; - hash ^= (u_int32_t)*p; + hash ^= (unsigned int)*p; } return (hash); } -/* - * ->ih_namelen == 0 indicates negative entry - */ -static int -idmap_cache_lookup(struct idmap *idmap, int type, char *name, u_int32_t *namelen, - uid_t *id) +int nfs_map_name_to_uid(struct nfs_server *server, const char *name, size_t namelen, __u32 *uid) { - u_int32_t hash; - struct idmap_hashent *he = NULL; - int insert = type & IDMAP_HASH_TYPE_INSERT; - int ret = -1; + struct idmap *idmap = server->idmap; - /* - * XXX technically, this is not needed, since we will always - * hold idmap_im_lock when altering the hash tables. but - * semantically that just hurts. - * - * XXX cache negative responses - */ - down(&idmap->idmap_hash_lock); + return nfs_idmap_id(idmap, &idmap->idmap_user_hash, name, namelen, uid); +} - if (*namelen > IDMAP_NAMESZ || *namelen == 0) - goto out; +int nfs_map_group_to_gid(struct nfs_server *server, const char *name, size_t namelen, __u32 *uid) +{ + struct idmap *idmap = server->idmap; - if (type & IDMAP_HASH_TYPE_NAME) { - hash = fnvhash32(name, *namelen) % IDMAP_HASH_SZ; - he = &idmap->idmap_name_hash[hash]; - - /* - * Testing he->ih_namelen == *namelen implicitly tests - * namelen != 0, and thus a non-negative entry. - */ - if (!insert && he->ih_namelen == *namelen && - memcmp(he->ih_name, name, *namelen) == 0) { - *id = he->ih_id; - ret = 0; - goto out; - } - } + return nfs_idmap_id(idmap, &idmap->idmap_group_hash, name, namelen, uid); +} - if (type & IDMAP_HASH_TYPE_ID) { - hash = fnvhash32(id, sizeof(*id)) % IDMAP_HASH_SZ; - he = &idmap->idmap_id_hash[hash]; - - if (!insert && *id == he->ih_id && he->ih_namelen != 0 && - *namelen >= he->ih_namelen) { - memcpy(name, he->ih_name, he->ih_namelen); - *namelen = he->ih_namelen; - ret = 0; - goto out; - } - } +int nfs_map_uid_to_name(struct nfs_server *server, __u32 uid, char *buf) +{ + struct idmap *idmap = server->idmap; - if (insert && he != NULL) { - he->ih_id = *id; - memcpy(he->ih_name, name, *namelen); - he->ih_namelen = *namelen; - ret = 0; - } + return nfs_idmap_name(idmap, &idmap->idmap_user_hash, uid, buf); +} +int nfs_map_gid_to_group(struct nfs_server *server, __u32 uid, char *buf) +{ + struct idmap *idmap = server->idmap; - out: - up(&idmap->idmap_hash_lock); - return (ret); + return nfs_idmap_name(idmap, &idmap->idmap_group_hash, uid, buf); } + diff -puN fs/nfs/nfs4xdr.c~nfs-06-fix_idmap2 fs/nfs/nfs4xdr.c --- 25/fs/nfs/nfs4xdr.c~nfs-06-fix_idmap2 2004-01-09 22:16:12.000000000 -0800 +++ 25-akpm/fs/nfs/nfs4xdr.c 2004-01-09 22:16:12.000000000 -0800 @@ -239,10 +239,10 @@ static int encode_attrs(struct xdr_stream *xdr, struct iattr *iap, struct nfs_server *server) { - char owner_name[256]; - char owner_group[256]; - int owner_namelen = sizeof(owner_name); - int owner_grouplen = sizeof(owner_group); + char owner_name[IDMAP_NAMESZ]; + char owner_group[IDMAP_NAMESZ]; + int owner_namelen = 0; + int owner_grouplen = 0; uint32_t *p; uint32_t *q; int len; @@ -265,9 +265,8 @@ encode_attrs(struct xdr_stream *xdr, str if (iap->ia_valid & ATTR_MODE) len += 4; if (iap->ia_valid & ATTR_UID) { - status = nfs_idmap_name(server, IDMAP_TYPE_USER, - iap->ia_uid, owner_name, &owner_namelen); - if (status < 0) { + owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name); + if (owner_namelen < 0) { printk(KERN_WARNING "nfs: couldn't resolve uid %d to string\n", iap->ia_uid); /* XXX */ @@ -278,9 +277,8 @@ encode_attrs(struct xdr_stream *xdr, str len += 4 + (XDR_QUADLEN(owner_namelen) << 2); } if (iap->ia_valid & ATTR_GID) { - status = nfs_idmap_name(server, IDMAP_TYPE_GROUP, - iap->ia_gid, owner_group, &owner_grouplen); - if (status < 0) { + owner_grouplen = nfs_map_gid_to_group(server, iap->ia_gid, owner_group); + if (owner_grouplen < 0) { printk(KERN_WARNING "nfs4: couldn't resolve gid %d to string\n", iap->ia_gid); strcpy(owner_group, "nobody"); @@ -1475,10 +1473,9 @@ decode_getattr(struct xdr_stream *xdr, s } READ_BUF(dummy32); len += (XDR_QUADLEN(dummy32) << 2); - if ((status = nfs_idmap_id(server, IDMAP_TYPE_USER, - (char *)p, dummy32, &nfp->uid)) == -1) { - dprintk("read_attrs: gss_get_num failed!\n"); - /* goto out; */ + if ((status = nfs_map_name_to_uid(server, (char *)p, dummy32, + &nfp->uid)) < 0) { + dprintk("read_attrs: name-to-uid mapping failed!\n"); nfp->uid = -2; } dprintk("read_attrs: uid=%d\n", (int)nfp->uid); @@ -1493,11 +1490,10 @@ decode_getattr(struct xdr_stream *xdr, s } READ_BUF(dummy32); len += (XDR_QUADLEN(dummy32) << 2); - if ((status = nfs_idmap_id(server, IDMAP_TYPE_GROUP, - (char *)p, dummy32, &nfp->gid)) == -1) { - dprintk("read_attrs: gss_get_num failed!\n"); + if ((status = nfs_map_group_to_gid(server, (char *)p, dummy32, + &nfp->gid)) < 0) { + dprintk("read_attrs: group-to-gid mapping failed!\n"); nfp->gid = -2; - /* goto out; */ } dprintk("read_attrs: gid=%d\n", (int)nfp->gid); } diff -puN include/linux/nfs_idmap.h~nfs-06-fix_idmap2 include/linux/nfs_idmap.h --- 25/include/linux/nfs_idmap.h~nfs-06-fix_idmap2 2004-01-09 22:16:12.000000000 -0800 +++ 25-akpm/include/linux/nfs_idmap.h 2004-01-09 22:16:12.000000000 -0800 @@ -52,18 +52,21 @@ #define IDMAP_STATUS_SUCCESS 0x08 struct idmap_msg { - u_int8_t im_type; - u_int8_t im_conv; - char im_name[IDMAP_NAMESZ]; - u_int32_t im_id; - u_int8_t im_status; + __u8 im_type; + __u8 im_conv; + char im_name[IDMAP_NAMESZ]; + __u32 im_id; + __u8 im_status; }; #ifdef __KERNEL__ void *nfs_idmap_new(struct nfs_server *); void nfs_idmap_delete(struct nfs_server *); -int nfs_idmap_id(struct nfs_server *, u_int8_t, char *, u_int, uid_t *); -int nfs_idmap_name(struct nfs_server *, u_int8_t, uid_t, char *, u_int *); + +int nfs_map_name_to_uid(struct nfs_server *, const char *, size_t, __u32 *); +int nfs_map_group_to_gid(struct nfs_server *, const char *, size_t, __u32 *); +int nfs_map_uid_to_name(struct nfs_server *, __u32, char *); +int nfs_map_gid_to_group(struct nfs_server *, __u32, char *); #endif /* __KERNEL__ */ #endif /* NFS_IDMAP_H */ _