On Thu, 14 Nov 2013 12:07:11 +1100
NeilBrown <neilb@xxxxxxx> 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 <neilb@xxxxxxx>
>
> 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);
Looks good.
Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
--
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
