Return-Path: linux-nfs-owner@vger.kernel.org Received: from mx1.redhat.com ([209.132.183.28]:6025 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755167Ab3KTVUx (ORCPT ); Wed, 20 Nov 2013 16:20:53 -0500 Message-ID: <528D27EB.60503@RedHat.com> Date: Wed, 20 Nov 2013 16:21:47 -0500 From: Steve Dickson MIME-Version: 1.0 To: NeilBrown CC: "Myklebust, Trond" , "J. Bruce Fields" , Charles Edward Lever , Linux NFS Mailing List Subject: Re: [PATCH - nfs-utils] gssd: always reply to rpc-pipe requests from kernel. References: <1384037221-7224-1-git-send-email-steved@redhat.com> <52811CBB.3070204@RedHat.com> <5281290B.6000201@RedHat.com> <20131112161135.25a487da@notabene.brown> <20131112161634.GC15060@fieldses.org> <20131113112346.3f5f3bd0@notabene.brown> <1384302651.15992.3.camel@leira.trondhjem.org> <20131113121333.2a16f646@notabene.brown> <1384306012.15992.9.camel@leira.trondhjem.org> <20131114120711.4043a60f@notabene.brown> In-Reply-To: <20131114120711.4043a60f@notabene.brown> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: On 13/11/13 20:07, NeilBrown wrote: > > Sometimes gssd will open a new rpc-pipe but never read requests from it > or reply to them. This causes the kernel to wait forever for a reply. > > In particular, if a filesystem is mounted by IP, and the IP has no > hostname recorded in /etc/hosts or DNS, then gssd will not listen to > requests and the mount will hang indefinitely. > > The comment in process_clnt_dir() for the "fail_keep_client:" branch > suggests that it is for the case where we couldn't open some > subdirectories. However it is currently also taken if reverse DNS > lookup fails (as well as some other lookup failures). Those failures > should not be treated the same as failure-to-open directories. > > So this patch causes a failure from read_service_info() to *not* be > reported by process_clnt_dir_files. This ensures that insert_clnt_poll() > will be called and requests will be handled. > > In handle_gssd_upcall, the current error path (taken when the mech is > not "krb5") does not reply to the upcall. This is wrong. A reply is > always appropriate. The only replies which aren't treated as > transient errors are EACCES and EKEYEXPIRED, so we return the former. > > If read_service_info() fails then ->servicename will be NULL which will > cause process_krb5_upcall() (quite reasonably) to become confused. So > in that case we don't even try to process the up-call but just reply > with EACCES. > > As clp->servicename==NULL is no longer treated as fatal, it is not > appropraite to use it to test if read_service_info() has been already > called on a client. Instread test clp->prog. > > Finally, the error path of read_service_info() will close 'fd' if it > isn't -1, so when we close it, we should set fd to -1. > > Signed-off-by: NeilBrown Committed (tag: nfs-utils-1-2-10-rc1) steved. > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c > index b48d1637cd36..58c2a282524f 100644 > --- a/utils/gssd/gssd_proc.c > +++ b/utils/gssd/gssd_proc.c > @@ -256,6 +256,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername, > if ((nbytes = read(fd, buf, INFOBUFLEN)) == -1) > goto fail; > close(fd); > + fd = -1; > buf[nbytes] = '\0'; > > numfields = sscanf(buf,"RPC server: %127s\n" > @@ -403,11 +404,10 @@ process_clnt_dir_files(struct clnt_info * clp) > return -1; > snprintf(info_file_name, sizeof(info_file_name), "%s/info", > clp->dirname); > - if ((clp->servicename == NULL) && > - read_service_info(info_file_name, &clp->servicename, > - &clp->servername, &clp->prog, &clp->vers, > - &clp->protocol, (struct sockaddr *) &clp->addr)) > - return -1; > + if (clp->prog == 0) > + read_service_info(info_file_name, &clp->servicename, > + &clp->servername, &clp->prog, &clp->vers, > + &clp->protocol, (struct sockaddr *) &clp->addr); > return 0; > } > > @@ -1320,11 +1320,14 @@ handle_gssd_upcall(struct clnt_info *clp) > } > } > > - if (strcmp(mech, "krb5") == 0) > + if (strcmp(mech, "krb5") == 0 && clp->servername) > process_krb5_upcall(clp, uid, clp->gssd_fd, target, service); > - else > - printerr(0, "WARNING: handle_gssd_upcall: " > - "received unknown gss mech '%s'\n", mech); > + else { > + if (clp->servername) > + printerr(0, "WARNING: handle_gssd_upcall: " > + "received unknown gss mech '%s'\n", mech); > + do_error_downcall(clp->gssd_fd, uid, -EACCES); > + } > > out: > free(lbuf); >