From: Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Re: [PATCH] sunrpc: on successful gss error pipe write, don't
	return error (try #2)
Date: Fri, 18 Dec 2009 14:05:39 -0500
Message-ID: <1261163139.3420.17.camel@localhost>
References: <1261153637-6209-1-git-send-email-jlayton@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: linux-nfs@vger.kernel.org, nfsv4@linux-nfs.org
To: Jeff Layton <jlayton@redhat.com>
In-Reply-To: <1261153637-6209-1-git-send-email-jlayton@redhat.com>
Sender: nfsv4-bounces@linux-nfs.org
Errors-To: nfsv4-bounces@linux-nfs.org

On Fri, 2009-12-18 at 11:27 -0500, Jeff Layton wrote: 
> When handling the gssd downcall, the kernel should distinguish between a
> successful downcall that contains an error code and a failed downcall
> (i.e. where the parsing failed or some other sort of problem occurred).
> 
> In the former case, gss_pipe_downcall should be returning the number of
> bytes written to the pipe instead of an error.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  net/sunrpc/auth_gss/auth_gss.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 3c3c50f..7afc8e2 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -645,6 +645,9 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
>  	if (IS_ERR(p)) {
>  		err = PTR_ERR(p);
>  		gss_msg->msg.errno = (err == -EAGAIN) ? -EAGAIN : -EACCES;
> +		/* special case: downcall was successful, but held an error */
> +		if (err == -EACCES)
> +			err = mlen;

That line immediately above your fix still looks wrong. The point is
that AFAICS, err is never going to be set to EAGAIN. It can be EFAULT,
ENOSYS, or ENOMEM, but it will never be EAGAIN...

I think we should rather reverse that test. Really, what we want to do,
is to set msg.errno to -EAGAIN for -EFAULT and -ENOMEM (and probably for
ENOSYS too), and then set it to -EACCES _only_ in the case where the
user was not authorised.

Trond