Re: [PATCH - nfs-utils] gssd: always reply to rpc-pipe requests from kernel.

[Steve Dickson <SteveD@redhat.com>]

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 <neilb@suse.de>
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);
>