NFSv4: Fix races in the legacy idmapper upcall

commit 51fd2eb52c0ca8275a906eed81878ef50ae94eb0 upstream.

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: e9ab41b620e4 ("NFSv4: Clean up the legacy idmapper upcall")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index f331866..ec6afd3 100644 (file)
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -561,22 +561,20 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
        return true;
 }
 
-static void
-nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
+static void nfs_idmap_complete_pipe_upcall(struct idmap_legacy_upcalldata *data,
+                                          int ret)
 {
-       struct key *authkey = idmap->idmap_upcall_data->authkey;
-
-       kfree(idmap->idmap_upcall_data);
-       idmap->idmap_upcall_data = NULL;
-       complete_request_key(authkey, ret);
-       key_put(authkey);
+       complete_request_key(data->authkey, ret);
+       key_put(data->authkey);
+       kfree(data);
 }
 
-static void
-nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
+static void nfs_idmap_abort_pipe_upcall(struct idmap *idmap,
+                                       struct idmap_legacy_upcalldata *data,
+                                       int ret)
 {
-       if (idmap->idmap_upcall_data != NULL)
-               nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
+       if (cmpxchg(&idmap->idmap_upcall_data, data, NULL) == data)
+               nfs_idmap_complete_pipe_upcall(data, ret);
 }
 
 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);
        if (ret < 0)
-               nfs_idmap_abort_pipe_upcall(idmap, ret);
+               nfs_idmap_abort_pipe_upcall(idmap, data, ret);
 
        return ret;
 out2:
@@ -669,6 +667,7 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
        struct request_key_auth *rka;
        struct rpc_inode *rpci = RPC_I(file_inode(filp));
        struct idmap *idmap = (struct idmap *)rpci->private;
+       struct idmap_legacy_upcalldata *data;
        struct key *authkey;
        struct idmap_msg im;
        size_t namelen_in;
@@ -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
         * 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;
 
-       authkey = idmap->idmap_upcall_data->authkey;
+       authkey = data->authkey;
        rka = get_request_key_auth(authkey);
 
        if (mlen != sizeof(im)) {
@@ -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) {
                ret = -EINVAL;
                goto out;
-}
+       }
 
-       ret = nfs_idmap_read_and_verify_message(&im,
-                       &idmap->idmap_upcall_data->idmap_msg,
-                       rka->target_key, authkey);
+       ret = nfs_idmap_read_and_verify_message(&im, &data->idmap_msg,
+                                               rka->target_key, authkey);
        if (ret >= 0) {
                key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
                ret = mlen;
        }
 
 out:
-       nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
+       nfs_idmap_complete_pipe_upcall(data, ret);
 out_noupcall:
        return ret;
 }
@@ -728,7 +727,7 @@ 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);
+               nfs_idmap_abort_pipe_upcall(idmap, data, msg->errno);
 }
 
 static void
@@ -736,8 +735,11 @@ idmap_release_pipe(struct inode *inode)
 {
        struct rpc_inode *rpci = RPC_I(inode);
        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)