Commit b18cba09 authored by minoura makoto's avatar minoura makoto Committed by Trond Myklebust

SUNRPC: ensure the matching upcall is in-flight upon downcall

Commit 9130b8db ("SUNRPC: allow for upcalls for the same uid
but different gss service") introduced `auth` argument to
__gss_find_upcall(), but in gss_pipe_downcall() it was left as NULL
since it (and auth->service) was not (yet) determined.

When multiple upcalls with the same uid and different service are
ongoing, it could happen that __gss_find_upcall(), which returns the
first match found in the pipe->in_downcall list, could not find the
correct gss_msg corresponding to the downcall we are looking for.
Moreover, it might return a msg which is not sent to rpc.gssd yet.

We could see mount.nfs process hung in D state with multiple mount.nfs
are executed in parallel.  The call trace below is of CentOS 7.9
kernel-3.10.0-1160.24.1.el7.x86_64 but we observed the same hang w/
elrepo kernel-ml-6.0.7-1.el7.

PID: 71258  TASK: ffff91ebd4be0000  CPU: 36  COMMAND: "mount.nfs"
 #0 [ffff9203ca3234f8] __schedule at ffffffffa3b8899f
 #1 [ffff9203ca323580] schedule at ffffffffa3b88eb9
 #2 [ffff9203ca323590] gss_cred_init at ffffffffc0355818 [auth_rpcgss]
 #3 [ffff9203ca323658] rpcauth_lookup_credcache at ffffffffc0421ebc
[sunrpc]
 #4 [ffff9203ca3236d8] gss_lookup_cred at ffffffffc0353633 [auth_rpcgss]
 #5 [ffff9203ca3236e8] rpcauth_lookupcred at ffffffffc0421581 [sunrpc]
 #6 [ffff9203ca323740] rpcauth_refreshcred at ffffffffc04223d3 [sunrpc]
 #7 [ffff9203ca3237a0] call_refresh at ffffffffc04103dc [sunrpc]
 #8 [ffff9203ca3237b8] __rpc_execute at ffffffffc041e1c9 [sunrpc]
 #9 [ffff9203ca323820] rpc_execute at ffffffffc0420a48 [sunrpc]

The scenario is like this. Let's say there are two upcalls for
services A and B, A -> B in pipe->in_downcall, B -> A in pipe->pipe.

When rpc.gssd reads pipe to get the upcall msg corresponding to
service B from pipe->pipe and then writes the response, in
gss_pipe_downcall the msg corresponding to service A will be picked
because only uid is used to find the msg and it is before the one for
B in pipe->in_downcall.  And the process waiting for the msg
corresponding to service A will be woken up.

Actual scheduing of that process might be after rpc.gssd processes the
next msg.  In rpc_pipe_generic_upcall it clears msg->errno (for A).
The process is scheduled to see gss_msg->ctx == NULL and
gss_msg->msg.errno == 0, therefore it cannot break the loop in
gss_create_upcall and is never woken up after that.

This patch adds a simple check to ensure that a msg which is not
sent to rpc.gssd yet is not chosen as the matching upcall upon
receiving a downcall.
Signed-off-by: default avatarminoura makoto <minoura@valinux.co.jp>
Signed-off-by: default avatarHiroshi Shimamoto <h-shimamoto@nec.com>
Tested-by: default avatarHiroshi Shimamoto <h-shimamoto@nec.com>
Cc: Trond Myklebust <trondmy@hammerspace.com>
Fixes: 9130b8db ("SUNRPC: allow for upcalls for same uid but different gss service")
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent 7fd461c4
...@@ -92,6 +92,11 @@ extern ssize_t rpc_pipe_generic_upcall(struct file *, struct rpc_pipe_msg *, ...@@ -92,6 +92,11 @@ extern ssize_t rpc_pipe_generic_upcall(struct file *, struct rpc_pipe_msg *,
char __user *, size_t); char __user *, size_t);
extern int rpc_queue_upcall(struct rpc_pipe *, struct rpc_pipe_msg *); extern int rpc_queue_upcall(struct rpc_pipe *, struct rpc_pipe_msg *);
/* returns true if the msg is in-flight, i.e., already eaten by the peer */
static inline bool rpc_msg_is_inflight(const struct rpc_pipe_msg *msg) {
return (msg->copied != 0 && list_empty(&msg->list));
}
struct rpc_clnt; struct rpc_clnt;
extern struct dentry *rpc_create_client_dir(struct dentry *, const char *, struct rpc_clnt *); extern struct dentry *rpc_create_client_dir(struct dentry *, const char *, struct rpc_clnt *);
extern int rpc_remove_client_dir(struct rpc_clnt *); extern int rpc_remove_client_dir(struct rpc_clnt *);
......
...@@ -302,7 +302,7 @@ __gss_find_upcall(struct rpc_pipe *pipe, kuid_t uid, const struct gss_auth *auth ...@@ -302,7 +302,7 @@ __gss_find_upcall(struct rpc_pipe *pipe, kuid_t uid, const struct gss_auth *auth
list_for_each_entry(pos, &pipe->in_downcall, list) { list_for_each_entry(pos, &pipe->in_downcall, list) {
if (!uid_eq(pos->uid, uid)) if (!uid_eq(pos->uid, uid))
continue; continue;
if (auth && pos->auth->service != auth->service) if (pos->auth->service != auth->service)
continue; continue;
refcount_inc(&pos->count); refcount_inc(&pos->count);
return pos; return pos;
...@@ -686,6 +686,21 @@ gss_create_upcall(struct gss_auth *gss_auth, struct gss_cred *gss_cred) ...@@ -686,6 +686,21 @@ gss_create_upcall(struct gss_auth *gss_auth, struct gss_cred *gss_cred)
return err; return err;
} }
static struct gss_upcall_msg *
gss_find_downcall(struct rpc_pipe *pipe, kuid_t uid)
{
struct gss_upcall_msg *pos;
list_for_each_entry(pos, &pipe->in_downcall, list) {
if (!uid_eq(pos->uid, uid))
continue;
if (!rpc_msg_is_inflight(&pos->msg))
continue;
refcount_inc(&pos->count);
return pos;
}
return NULL;
}
#define MSG_BUF_MAXSIZE 1024 #define MSG_BUF_MAXSIZE 1024
static ssize_t static ssize_t
...@@ -732,7 +747,7 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -732,7 +747,7 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
err = -ENOENT; err = -ENOENT;
/* Find a matching upcall */ /* Find a matching upcall */
spin_lock(&pipe->lock); spin_lock(&pipe->lock);
gss_msg = __gss_find_upcall(pipe, uid, NULL); gss_msg = gss_find_downcall(pipe, uid);
if (gss_msg == NULL) { if (gss_msg == NULL) {
spin_unlock(&pipe->lock); spin_unlock(&pipe->lock);
goto err_put_ctx; goto err_put_ctx;
......
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