On Fri, 2009-12-18 at 08:56 -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@xxxxxxxxxx>
> ---
> net/sunrpc/auth_gss/auth_gss.c | 21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index 3c3c50f..03cc5a4 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -184,7 +184,8 @@ gss_alloc_context(void)
>
> #define GSSD_MIN_TIMEOUT (60 * 60)
> static const void *
> -gss_fill_context(const void *p, const void *end, struct gss_cl_ctx *ctx, struct gss_api_mech *gm)
> +gss_fill_context(const void *p, const void *end, struct gss_cl_ctx *ctx,
> + struct gss_api_mech *gm, ssize_t *downcall_err)
> {
> const void *q;
> unsigned int seclen;
> @@ -208,6 +209,7 @@ gss_fill_context(const void *p, const void *end, struct gss_cl_ctx *ctx, struct
> if (ctx->gc_win == 0) {
> /* in which case, p points to an error code which we ignore */
> p = ERR_PTR(-EACCES);
> + *downcall_err = -EACCES;
> goto err;
> }
> /* copy the opaque wire context */
> @@ -641,10 +643,21 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> list_del_init(&gss_msg->list);
> spin_unlock(&inode->i_lock);
>
> - p = gss_fill_context(p, end, ctx, gss_msg->auth->mech);
> + err = 0;
> + p = gss_fill_context(p, end, ctx, gss_msg->auth->mech, &err);
> if (IS_ERR(p)) {
> - err = PTR_ERR(p);
> - gss_msg->msg.errno = (err == -EAGAIN) ? -EAGAIN : -EACCES;
> + /*
> + * a non-zero downcall_err indicates that downcall write was
> + * OK, but contained a zero gc_win (and hence an error code).
> + */
> + if (err) {
> + gss_msg->msg.errno = err;
> + err = mlen;
> + } else {
> + err = PTR_ERR(p);
> + gss_msg->msg.errno = (err == -EAGAIN) ?
> + -EAGAIN : -EACCES;
> + }
> goto err_release_msg;
> }
> gss_msg->ctx = gss_get_ctx(ctx);
Is this extra parameter really necessary? Can't we just distinguish
between EACCES, which means that the downcall was successful, but
contained an error, and EFAULT/ENOMEM/ENOSYS, which are context creation
errors.
BTW: while looking at this, I spotted a nasty bug in
gss_import_sec_context_kerberos(). If the kzalloc() call fails, we will
return a random error code since 'p' still points to a valid memory
location...
Trond
--
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
