Commit 51fd2eb5 authored by Trond Myklebust's avatar Trond Myklebust

NFSv4: Fix races in the legacy idmapper upcall

nfs_idmap_instantiate() will cause the process that is waiting in
request_key_with_auxdata() to wake up and exit. If there is a second
process waiting for the idmap->idmap_mutex, then it may wake up and
start a new call to request_key_with_auxdata(). If the call to
idmap_pipe_downcall() from the first process has not yet finished
calling nfs_idmap_complete_pipe_upcall_locked(), then we may end up
triggering the WARN_ON_ONCE() in nfs_idmap_prepare_pipe_upcall().

The fix is to ensure that we clear idmap->idmap_upcall_data before
calling nfs_idmap_instantiate().

Fixes: e9ab41b6 ("NFSv4: Clean up the legacy idmapper upcall")
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent 940261a1
...@@ -561,22 +561,20 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap, ...@@ -561,22 +561,20 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
return true; return true;
} }
static void static void nfs_idmap_complete_pipe_upcall(struct idmap_legacy_upcalldata *data,
nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret) int ret)
{ {
struct key *authkey = idmap->idmap_upcall_data->authkey; complete_request_key(data->authkey, ret);
key_put(data->authkey);
kfree(idmap->idmap_upcall_data); kfree(data);
idmap->idmap_upcall_data = NULL;
complete_request_key(authkey, ret);
key_put(authkey);
} }
static void static void nfs_idmap_abort_pipe_upcall(struct idmap *idmap,
nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret) struct idmap_legacy_upcalldata *data,
int ret)
{ {
if (idmap->idmap_upcall_data != NULL) if (cmpxchg(&idmap->idmap_upcall_data, data, NULL) == data)
nfs_idmap_complete_pipe_upcall_locked(idmap, ret); nfs_idmap_complete_pipe_upcall(data, ret);
} }
static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux) static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
...@@ -613,7 +611,7 @@ static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux) ...@@ -613,7 +611,7 @@ static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
ret = rpc_queue_upcall(idmap->idmap_pipe, msg); ret = rpc_queue_upcall(idmap->idmap_pipe, msg);
if (ret < 0) if (ret < 0)
nfs_idmap_abort_pipe_upcall(idmap, ret); nfs_idmap_abort_pipe_upcall(idmap, data, ret);
return ret; return ret;
out2: out2:
...@@ -669,6 +667,7 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -669,6 +667,7 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
struct request_key_auth *rka; struct request_key_auth *rka;
struct rpc_inode *rpci = RPC_I(file_inode(filp)); struct rpc_inode *rpci = RPC_I(file_inode(filp));
struct idmap *idmap = (struct idmap *)rpci->private; struct idmap *idmap = (struct idmap *)rpci->private;
struct idmap_legacy_upcalldata *data;
struct key *authkey; struct key *authkey;
struct idmap_msg im; struct idmap_msg im;
size_t namelen_in; size_t namelen_in;
...@@ -678,10 +677,11 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -678,10 +677,11 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
* will have been woken up and someone else may now have used * 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. * idmap_key_cons - so after this point we may no longer touch it.
*/ */
if (idmap->idmap_upcall_data == NULL) data = xchg(&idmap->idmap_upcall_data, NULL);
if (data == NULL)
goto out_noupcall; goto out_noupcall;
authkey = idmap->idmap_upcall_data->authkey; authkey = data->authkey;
rka = get_request_key_auth(authkey); rka = get_request_key_auth(authkey);
if (mlen != sizeof(im)) { if (mlen != sizeof(im)) {
...@@ -703,18 +703,17 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -703,18 +703,17 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
if (namelen_in == 0 || namelen_in == IDMAP_NAMESZ) { if (namelen_in == 0 || namelen_in == IDMAP_NAMESZ) {
ret = -EINVAL; ret = -EINVAL;
goto out; goto out;
} }
ret = nfs_idmap_read_and_verify_message(&im, ret = nfs_idmap_read_and_verify_message(&im, &data->idmap_msg,
&idmap->idmap_upcall_data->idmap_msg, rka->target_key, authkey);
rka->target_key, authkey);
if (ret >= 0) { if (ret >= 0) {
key_set_timeout(rka->target_key, nfs_idmap_cache_timeout); key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
ret = mlen; ret = mlen;
} }
out: out:
nfs_idmap_complete_pipe_upcall_locked(idmap, ret); nfs_idmap_complete_pipe_upcall(data, ret);
out_noupcall: out_noupcall:
return ret; return ret;
} }
...@@ -728,7 +727,7 @@ idmap_pipe_destroy_msg(struct rpc_pipe_msg *msg) ...@@ -728,7 +727,7 @@ idmap_pipe_destroy_msg(struct rpc_pipe_msg *msg)
struct idmap *idmap = data->idmap; struct idmap *idmap = data->idmap;
if (msg->errno) if (msg->errno)
nfs_idmap_abort_pipe_upcall(idmap, msg->errno); nfs_idmap_abort_pipe_upcall(idmap, data, msg->errno);
} }
static void static void
...@@ -736,8 +735,11 @@ idmap_release_pipe(struct inode *inode) ...@@ -736,8 +735,11 @@ idmap_release_pipe(struct inode *inode)
{ {
struct rpc_inode *rpci = RPC_I(inode); struct rpc_inode *rpci = RPC_I(inode);
struct idmap *idmap = (struct idmap *)rpci->private; struct idmap *idmap = (struct idmap *)rpci->private;
struct idmap_legacy_upcalldata *data;
nfs_idmap_abort_pipe_upcall(idmap, -EPIPE); data = xchg(&idmap->idmap_upcall_data, NULL);
if (data)
nfs_idmap_complete_pipe_upcall(data, -EPIPE);
} }
int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_t namelen, kuid_t *uid) int nfs_map_name_to_uid(const struct nfs_server *server, const char *name, size_t namelen, kuid_t *uid)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment