On Sat, Jan 04, 2014 at 09:11:05AM -0500, Jeff Layton wrote:
> On Sat, 4 Jan 2014 07:18:04 -0500
> Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> > Currently, the write_gssp code will change the variable and wake up any
> > waiters waiting on that change. It then goes and tries to set the
> > gssp_clnt. This is racy -- a task waiting on the set_gss_proxy call may
> > end up waking up and then subsequently finding that the gss_clnt isn't
> > there yet and end up not using it even though it'll soon be ready.
> >
> > This patch reverses the order of operations. The gssp_clnt is created
> > first, and the variable change is done only if that succeeds.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 1b94a9c..60dc370 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -1317,10 +1317,10 @@ static ssize_t write_gssp(struct file *file, const char __user *buf,
> > return res;
> > if (i != 1)
> > return -EINVAL;
> > - res = set_gss_proxy(net, 1);
> > + res = set_gssp_clnt(net);
> > if (res)
> > return res;
> > - res = set_gssp_clnt(net);
> > + res = set_gss_proxy(net, 1);
> > if (res)
> > return res;
> > return count;
>
> Sorry, I forgot to update the patch description on this one. There is
> still a race here after patch #1, but it goes something like this:
>
> A nfsd thread will call use_gss_proxy and find it set to '1'. It'll
> then go and attempt and upcall, but since gssp_clnt is still NULL,
> gssp_call will just return -EIO.
>
> The patch is still the same however. Bruce, let me know if you want me
> to resend with a fixed commit msg.
No problem. Applying as follows.--b.
commit 32d6805adfc998def6b77ab95f35f63ad07cd043
Author: Jeff Layton <jlayton@xxxxxxxxxx>
Date: Sat Jan 4 07:18:04 2014 -0500
sunrpc: fix potential race between setting use_gss_proxy and the upcall rpc_clnt
An nfsd thread can call use_gss_proxy and find it set to '1' but find
gssp_clnt still NULL, so that when it attempts the upcall the result
will be an unnecessary -EIO.
So, ensure that gssp_clnt is created first, and set the use_gss_proxy
variable only if that succeeds.
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 084c87e..a80af65 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1317,10 +1317,10 @@ static ssize_t write_gssp(struct file *file, const char __user *buf,
return res;
if (i != 1)
return -EINVAL;
- res = set_gss_proxy(net, 1);
+ res = set_gssp_clnt(net);
if (res)
return res;
- res = set_gssp_clnt(net);
+ res = set_gss_proxy(net, 1);
if (res)
return res;
return count;
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
