From patchwork Sun Jan 5 20:21:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "J. Bruce Fields" X-Patchwork-Id: 3434911 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3B2B3C02DC for ; Sun, 5 Jan 2014 20:22:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1267620136 for ; Sun, 5 Jan 2014 20:22:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3281D200E3 for ; Sun, 5 Jan 2014 20:22:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751782AbaAEUV7 (ORCPT ); Sun, 5 Jan 2014 15:21:59 -0500 Received: from fieldses.org ([174.143.236.118]:48209 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751669AbaAEUV6 (ORCPT ); Sun, 5 Jan 2014 15:21:58 -0500 Received: from bfields by fieldses.org with local (Exim 4.76) (envelope-from ) id 1VzuCr-00061g-NO; Sun, 05 Jan 2014 15:21:57 -0500 Date: Sun, 5 Jan 2014 15:21:57 -0500 From: "J. Bruce Fields" To: Jeff Layton Cc: linux-nfs@vger.kernel.org, ssorce@redhat.com, neilb@suse.de Subject: Re: [PATCH 2/3] sunrpc: fix potential race between setting use_gss_proxy and the upcall rpc_clnt Message-ID: <20140105202157.GB22918@fieldses.org> References: <1388837885-8312-1-git-send-email-jlayton@redhat.com> <1388837885-8312-3-git-send-email-jlayton@redhat.com> <20140104091105.4dc2a971@tlielax.poochiereds.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140104091105.4dc2a971@tlielax.poochiereds.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Jan 04, 2014 at 09:11:05AM -0500, Jeff Layton wrote: > On Sat, 4 Jan 2014 07:18:04 -0500 > Jeff Layton 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 > > --- > > 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 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 Signed-off-by: J. Bruce Fields --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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;