* [PATCH 1/2] NFSv4: Fix the legacy idmapper upcall locking
@ 2012-09-27 20:31 Trond Myklebust
2012-09-27 20:31 ` [PATCH 2/2] NFSv4: Ensure that idmap_pipe_downcall sanity-checks the downcall data Trond Myklebust
0 siblings, 1 reply; 2+ messages in thread
From: Trond Myklebust @ 2012-09-27 20:31 UTC (permalink / raw)
To: linux-nfs; +Cc: Bryan Schumaker
We must not touch the idmap->idmap_key_cons field when a downcall
is in progress. Set up a mutex and helper functions to ensure that
we wait until the downcall is finished.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Bryan Schumaker <bjschuma@netapp.com>
---
fs/nfs/idmap.c | 65 ++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 45 insertions(+), 20 deletions(-)
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index a850079..7c7072a 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -59,6 +59,7 @@ struct idmap {
struct rpc_pipe *idmap_pipe;
struct key_construction *idmap_key_cons;
struct mutex idmap_mutex;
+ struct mutex idmap_downcall_mutex;
};
struct idmap_legacy_upcalldata {
@@ -330,7 +331,6 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
ret = nfs_idmap_request_key(&key_type_id_resolver_legacy,
name, namelen, type, data,
data_size, idmap);
- idmap->idmap_key_cons = NULL;
mutex_unlock(&idmap->idmap_mutex);
}
return ret;
@@ -485,6 +485,7 @@ nfs_idmap_new(struct nfs_client *clp)
}
idmap->idmap_pipe = pipe;
mutex_init(&idmap->idmap_mutex);
+ mutex_init(&idmap->idmap_downcall_mutex);
clp->cl_idmap = idmap;
return 0;
@@ -665,6 +666,32 @@ out:
return ret;
}
+static void
+nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
+ struct key_construction *cons)
+{
+ mutex_lock(&idmap->idmap_downcall_mutex);
+ WARN_ON_ONCE(idmap->idmap_key_cons != NULL);
+ idmap->idmap_key_cons = cons;
+ mutex_unlock(&idmap->idmap_downcall_mutex);
+}
+
+static void
+nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
+{
+ complete_request_key(idmap->idmap_key_cons, ret);
+ idmap->idmap_key_cons = NULL;
+}
+
+static void
+nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
+{
+ mutex_lock(&idmap->idmap_downcall_mutex);
+ if (idmap->idmap_key_cons != NULL)
+ nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
+ mutex_unlock(&idmap->idmap_downcall_mutex);
+}
+
static int nfs_idmap_legacy_upcall(struct key_construction *cons,
const char *op,
void *aux)
@@ -689,17 +716,14 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
if (ret < 0)
goto out2;
- BUG_ON(idmap->idmap_key_cons != NULL);
- idmap->idmap_key_cons = cons;
-
+ nfs_idmap_prepare_pipe_upcall(idmap, cons);
ret = rpc_queue_upcall(idmap->idmap_pipe, msg);
- if (ret < 0)
- goto out3;
+ if (ret < 0) {
+ nfs_idmap_abort_pipe_upcall(idmap, ret);
+ kfree(data);
+ }
return ret;
-
-out3:
- idmap->idmap_key_cons = NULL;
out2:
kfree(data);
out1:
@@ -740,14 +764,16 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
struct key_construction *cons;
struct idmap_msg im;
size_t namelen_in;
- int ret;
+ int ret = -ENOKEY;
/* If instantiation is successful, anyone waiting for key construction
* will have been woken up and someone else may now have used
* idmap_key_cons - so after this point we may no longer touch it.
*/
- cons = ACCESS_ONCE(idmap->idmap_key_cons);
- idmap->idmap_key_cons = NULL;
+ mutex_lock(&idmap->idmap_downcall_mutex);
+ cons = idmap->idmap_key_cons;
+ if (cons == NULL)
+ goto out_unlock;
if (mlen != sizeof(im)) {
ret = -ENOSPC;
@@ -777,7 +803,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
}
out:
- complete_request_key(cons, ret);
+ nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
+out_unlock:
+ mutex_unlock(&idmap->idmap_downcall_mutex);
return ret;
}
@@ -788,12 +816,8 @@ idmap_pipe_destroy_msg(struct rpc_pipe_msg *msg)
struct idmap_legacy_upcalldata,
pipe_msg);
struct idmap *idmap = data->idmap;
- struct key_construction *cons;
- if (msg->errno) {
- cons = ACCESS_ONCE(idmap->idmap_key_cons);
- idmap->idmap_key_cons = NULL;
- complete_request_key(cons, msg->errno);
- }
+ if (msg->errno)
+ nfs_idmap_abort_pipe_upcall(idmap, msg->errno);
/* Free memory allocated in nfs_idmap_legacy_upcall() */
kfree(data);
}
@@ -803,7 +827,8 @@ idmap_release_pipe(struct inode *inode)
{
struct rpc_inode *rpci = RPC_I(inode);
struct idmap *idmap = (struct idmap *)rpci->private;
- idmap->idmap_key_cons = NULL;
+
+ nfs_idmap_abort_pipe_upcall(idmap, -EPIPE);
}
int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_t namelen, __u32 *uid)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH 2/2] NFSv4: Ensure that idmap_pipe_downcall sanity-checks the downcall data
2012-09-27 20:31 [PATCH 1/2] NFSv4: Fix the legacy idmapper upcall locking Trond Myklebust
@ 2012-09-27 20:31 ` Trond Myklebust
0 siblings, 0 replies; 2+ messages in thread
From: Trond Myklebust @ 2012-09-27 20:31 UTC (permalink / raw)
To: linux-nfs; +Cc: Bryan Schumaker
Use the idmapper upcall data to verify that the legacy idmapper daemon
is indeed responding to an upcall that we sent.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Bryan Schumaker <bjschuma@netapp.com>
---
fs/nfs/idmap.c | 62 +++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 7c7072a..34d3072 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -55,19 +55,20 @@
static const struct cred *id_resolver_cache;
static struct key_type key_type_id_resolver_legacy;
-struct idmap {
- struct rpc_pipe *idmap_pipe;
- struct key_construction *idmap_key_cons;
- struct mutex idmap_mutex;
- struct mutex idmap_downcall_mutex;
-};
-
struct idmap_legacy_upcalldata {
struct rpc_pipe_msg pipe_msg;
struct idmap_msg idmap_msg;
+ struct key_construction *key_cons;
struct idmap *idmap;
};
+struct idmap {
+ struct rpc_pipe *idmap_pipe;
+ struct idmap_legacy_upcalldata *idmap_upcall_data;
+ struct mutex idmap_mutex;
+ struct mutex idmap_downcall_mutex;
+};
+
/**
* nfs_fattr_init_names - initialise the nfs_fattr owner_name/group_name fields
* @fattr: fully initialised struct nfs_fattr
@@ -668,26 +669,27 @@ out:
static void
nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
- struct key_construction *cons)
+ struct idmap_legacy_upcalldata *data)
{
mutex_lock(&idmap->idmap_downcall_mutex);
- WARN_ON_ONCE(idmap->idmap_key_cons != NULL);
- idmap->idmap_key_cons = cons;
+ WARN_ON_ONCE(idmap->idmap_upcall_data != NULL);
+ idmap->idmap_upcall_data = data;
mutex_unlock(&idmap->idmap_downcall_mutex);
}
static void
nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
{
- complete_request_key(idmap->idmap_key_cons, ret);
- idmap->idmap_key_cons = NULL;
+ complete_request_key(idmap->idmap_upcall_data->key_cons, ret);
+ kfree(idmap->idmap_upcall_data);
+ idmap->idmap_upcall_data = NULL;
}
static void
nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
{
mutex_lock(&idmap->idmap_downcall_mutex);
- if (idmap->idmap_key_cons != NULL)
+ if (idmap->idmap_upcall_data != NULL)
nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
mutex_unlock(&idmap->idmap_downcall_mutex);
}
@@ -716,12 +718,10 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
if (ret < 0)
goto out2;
- nfs_idmap_prepare_pipe_upcall(idmap, cons);
+ nfs_idmap_prepare_pipe_upcall(idmap, data);
ret = rpc_queue_upcall(idmap->idmap_pipe, msg);
- if (ret < 0) {
+ if (ret < 0)
nfs_idmap_abort_pipe_upcall(idmap, ret);
- kfree(data);
- }
return ret;
out2:
@@ -738,21 +738,32 @@ static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *dat
authkey);
}
-static int nfs_idmap_read_message(struct idmap_msg *im, struct key *key, struct key *authkey)
+static int nfs_idmap_read_and_verify_message(struct idmap_msg *im,
+ struct idmap_msg *upcall,
+ struct key *key, struct key *authkey)
{
char id_str[NFS_UINT_MAXLEN];
- int ret = -EINVAL;
+ int ret = -ENOKEY;
+ /* ret = -ENOKEY */
+ if (upcall->im_type != im->im_type || upcall->im_conv != im->im_conv)
+ goto out;
switch (im->im_conv) {
case IDMAP_CONV_NAMETOID:
+ if (strcmp(upcall->im_name, im->im_name) != 0)
+ break;
sprintf(id_str, "%d", im->im_id);
ret = nfs_idmap_instantiate(key, authkey, id_str);
break;
case IDMAP_CONV_IDTONAME:
+ if (upcall->im_id != im->im_id)
+ break;
ret = nfs_idmap_instantiate(key, authkey, im->im_name);
break;
+ default:
+ ret = -EINVAL;
}
-
+out:
return ret;
}
@@ -771,10 +782,11 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
* idmap_key_cons - so after this point we may no longer touch it.
*/
mutex_lock(&idmap->idmap_downcall_mutex);
- cons = idmap->idmap_key_cons;
- if (cons == NULL)
+ if (idmap->idmap_upcall_data == NULL)
goto out_unlock;
+ cons = idmap->idmap_upcall_data->key_cons;
+
if (mlen != sizeof(im)) {
ret = -ENOSPC;
goto out;
@@ -796,7 +808,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
goto out;
}
- ret = nfs_idmap_read_message(&im, cons->key, cons->authkey);
+ ret = nfs_idmap_read_and_verify_message(&im,
+ &idmap->idmap_upcall_data->idmap_msg,
+ cons->key, cons->authkey);
if (ret >= 0) {
key_set_timeout(cons->key, nfs_idmap_cache_timeout);
ret = mlen;
@@ -818,8 +832,6 @@ idmap_pipe_destroy_msg(struct rpc_pipe_msg *msg)
struct idmap *idmap = data->idmap;
if (msg->errno)
nfs_idmap_abort_pipe_upcall(idmap, msg->errno);
- /* Free memory allocated in nfs_idmap_legacy_upcall() */
- kfree(data);
}
static void
--
1.7.11.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-09-27 20:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27 20:31 [PATCH 1/2] NFSv4: Fix the legacy idmapper upcall locking Trond Myklebust
2012-09-27 20:31 ` [PATCH 2/2] NFSv4: Ensure that idmap_pipe_downcall sanity-checks the downcall data Trond Myklebust
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).