linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* miscellaneous gss-proxy & krb5 fixes for 3.13
@ 2013-10-10 15:14 J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases J. Bruce Fields
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 15:14 UTC (permalink / raw)
  To: linux-nfs

These are some (what look like mostly minor) fixes prompted by some
static checker results from Andi Kleen.  I'm planning to queue them up
for 3.13.

--b.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases
  2013-10-10 15:14 miscellaneous gss-proxy & krb5 fixes for 3.13 J. Bruce Fields
@ 2013-10-10 15:15 ` J. Bruce Fields
  2013-10-10 15:35   ` Simo Sorce
  2013-10-10 15:15 ` [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall J. Bruce Fields
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 15:15 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields, Simo Sorce

From: "J. Bruce Fields" <bfields@redhat.com>

We depend on the xdr decoder to set this pointer, but if we error out
before we decode this piece it could be left NULL.

I think this is probably tough to hit without a buggy gss-proxy.

Reported-by: Andi Kleen <andi@firstfloor.org>
Cc: Simo Sorce <simo@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 net/sunrpc/auth_gss/gss_rpc_upcall.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index f1eb0d1..458f85e 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -298,7 +298,8 @@ int gssp_accept_sec_context_upcall(struct net *net,
 	if (res.context_handle) {
 		data->out_handle = rctxh.exported_context_token;
 		data->mech_oid.len = rctxh.mech.len;
-		memcpy(data->mech_oid.data, rctxh.mech.data,
+		if (rctxh.mech.data)
+			memcpy(data->mech_oid.data, rctxh.mech.data,
 						data->mech_oid.len);
 		client_name = rctxh.src_name.display_name;
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall
  2013-10-10 15:14 miscellaneous gss-proxy & krb5 fixes for 3.13 J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases J. Bruce Fields
@ 2013-10-10 15:15 ` J. Bruce Fields
  2013-10-10 15:37   ` Simo Sorce
  2013-10-10 15:15 ` [PATCH 3/4] svcrpc: handle some gssproxy encoding errors J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 4/4] gss_krb5: document that we ignore sequence number J. Bruce Fields
  3 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 15:15 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields, Simo Sorce

From: "J. Bruce Fields" <bfields@redhat.com>

For every other problem here we bail out with an error, but here for
some reason we're setting a negative cache entry (with, note, an
undefined expiry).

It seems simplest just to bail out in the same way as we do in other
cases.

Cc: Simo Sorce <simo@redhat.com>
Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 09fb638..008cdad 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1167,8 +1167,8 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
 	if (!ud->found_creds) {
 		/* userspace seem buggy, we should always get at least a
 		 * mapping to nobody */
-		dprintk("RPC:       No creds found, marking Negative!\n");
-		set_bit(CACHE_NEGATIVE, &rsci.h.flags);
+		dprintk("RPC:       No creds found!\n");
+		goto out;
 	} else {
 
 		/* steal creds */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] svcrpc: handle some gssproxy encoding errors
  2013-10-10 15:14 miscellaneous gss-proxy & krb5 fixes for 3.13 J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall J. Bruce Fields
@ 2013-10-10 15:15 ` J. Bruce Fields
  2013-10-10 15:15 ` [PATCH 4/4] gss_krb5: document that we ignore sequence number J. Bruce Fields
  3 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 15:15 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 net/sunrpc/auth_gss/gss_rpc_xdr.c |   29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index f0f78c5..1ec19f6 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -559,6 +559,8 @@ static int gssx_enc_cred(struct xdr_stream *xdr,
 
 	/* cred->elements */
 	err = dummy_enc_credel_array(xdr, &cred->elements);
+	if (err)
+		return err;
 
 	/* cred->cred_handle_reference */
 	err = gssx_enc_buffer(xdr, &cred->cred_handle_reference);
@@ -740,22 +742,20 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req,
 		goto done;
 
 	/* arg->context_handle */
-	if (arg->context_handle) {
+	if (arg->context_handle)
 		err = gssx_enc_ctx(xdr, arg->context_handle);
-		if (err)
-			goto done;
-	} else {
+	else
 		err = gssx_enc_bool(xdr, 0);
-	}
+	if (err)
+		goto done;
 
 	/* arg->cred_handle */
-	if (arg->cred_handle) {
+	if (arg->cred_handle)
 		err = gssx_enc_cred(xdr, arg->cred_handle);
-		if (err)
-			goto done;
-	} else {
+	else
 		err = gssx_enc_bool(xdr, 0);
-	}
+	if (err)
+		goto done;
 
 	/* arg->input_token */
 	err = gssx_enc_in_token(xdr, &arg->input_token);
@@ -763,13 +763,12 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req,
 		goto done;
 
 	/* arg->input_cb */
-	if (arg->input_cb) {
+	if (arg->input_cb)
 		err = gssx_enc_cb(xdr, arg->input_cb);
-		if (err)
-			goto done;
-	} else {
+	else
 		err = gssx_enc_bool(xdr, 0);
-	}
+	if (err)
+		goto done;
 
 	err = gssx_enc_bool(xdr, arg->ret_deleg_cred);
 	if (err)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] gss_krb5: document that we ignore sequence number
  2013-10-10 15:14 miscellaneous gss-proxy & krb5 fixes for 3.13 J. Bruce Fields
                   ` (2 preceding siblings ...)
  2013-10-10 15:15 ` [PATCH 3/4] svcrpc: handle some gssproxy encoding errors J. Bruce Fields
@ 2013-10-10 15:15 ` J. Bruce Fields
  3 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 15:15 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

A couple times recently somebody has noticed that we're ignoring a
sequence number here and wondered whether there's a bug.

In fact, there's not.  Thanks to Andy Adamson for pointing out a useful
explanation in rfc 2203.  Add comments citing that rfc, and remove
"seqnum" to prevent static checkers complaining about unused variables.

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 net/sunrpc/auth_gss/gss_krb5_unseal.c |    8 ++++----
 net/sunrpc/auth_gss/gss_krb5_wrap.c   |    6 ++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_unseal.c b/net/sunrpc/auth_gss/gss_krb5_unseal.c
index 6cd930f..6c981dd 100644
--- a/net/sunrpc/auth_gss/gss_krb5_unseal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_unseal.c
@@ -150,7 +150,6 @@ gss_verify_mic_v2(struct krb5_ctx *ctx,
 	struct xdr_netobj cksumobj = {.len = sizeof(cksumdata),
 				      .data = cksumdata};
 	s32 now;
-	u64 seqnum;
 	u8 *ptr = read_token->data;
 	u8 *cksumkey;
 	u8 flags;
@@ -197,9 +196,10 @@ gss_verify_mic_v2(struct krb5_ctx *ctx,
 	if (now > ctx->endtime)
 		return GSS_S_CONTEXT_EXPIRED;
 
-	/* do sequencing checks */
-
-	seqnum = be64_to_cpup((__be64 *)ptr + 8);
+	/*
+	 * NOTE: the sequence number at ptr + 8 is skipped, rpcsec_gss
+	 * doesn't want it checked; see page 6 of rfc 2203.
+	 */
 
 	return GSS_S_COMPLETE;
 }
diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index 1da52d1..5040a46 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -489,7 +489,6 @@ static u32
 gss_unwrap_kerberos_v2(struct krb5_ctx *kctx, int offset, struct xdr_buf *buf)
 {
 	s32		now;
-	u64		seqnum;
 	u8		*ptr;
 	u8		flags = 0x00;
 	u16		ec, rrc;
@@ -525,7 +524,10 @@ gss_unwrap_kerberos_v2(struct krb5_ctx *kctx, int offset, struct xdr_buf *buf)
 	ec = be16_to_cpup((__be16 *)(ptr + 4));
 	rrc = be16_to_cpup((__be16 *)(ptr + 6));
 
-	seqnum = be64_to_cpup((__be64 *)(ptr + 8));
+	/*
+	 * NOTE: the sequence number at ptr + 8 is skipped, rpcsec_gss
+	 * doesn't want it checked; see page 6 of rfc 2203.
+	 */
 
 	if (rrc != 0)
 		rotate_left(offset + 16, buf, rrc);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases
  2013-10-10 15:15 ` [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases J. Bruce Fields
@ 2013-10-10 15:35   ` Simo Sorce
  0 siblings, 0 replies; 10+ messages in thread
From: Simo Sorce @ 2013-10-10 15:35 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Thu, 2013-10-10 at 11:15 -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> We depend on the xdr decoder to set this pointer, but if we error out
> before we decode this piece it could be left NULL.
> 
> I think this is probably tough to hit without a buggy gss-proxy.
> 
> Reported-by: Andi Kleen <andi@firstfloor.org>
> Cc: Simo Sorce <simo@redhat.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  net/sunrpc/auth_gss/gss_rpc_upcall.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> index f1eb0d1..458f85e 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> @@ -298,7 +298,8 @@ int gssp_accept_sec_context_upcall(struct net *net,
>  	if (res.context_handle) {
>  		data->out_handle = rctxh.exported_context_token;
>  		data->mech_oid.len = rctxh.mech.len;
> -		memcpy(data->mech_oid.data, rctxh.mech.data,
> +		if (rctxh.mech.data)
> +			memcpy(data->mech_oid.data, rctxh.mech.data,
>  						data->mech_oid.len);
>  		client_name = rctxh.src_name.display_name;
>  	}

Reviewed-by: Simo Sorce <simo@redhat.com>

-- 
Simo Sorce * Red Hat, Inc * New York


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall
  2013-10-10 15:15 ` [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall J. Bruce Fields
@ 2013-10-10 15:37   ` Simo Sorce
  2013-10-10 19:23     ` J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: Simo Sorce @ 2013-10-10 15:37 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Thu, 2013-10-10 at 11:15 -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> For every other problem here we bail out with an error, but here for
> some reason we're setting a negative cache entry (with, note, an
> undefined expiry).
> 
> It seems simplest just to bail out in the same way as we do in other
> cases.
> 
> Cc: Simo Sorce <simo@redhat.com>
> Reported-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  net/sunrpc/auth_gss/svcauth_gss.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 09fb638..008cdad 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -1167,8 +1167,8 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
>  	if (!ud->found_creds) {
>  		/* userspace seem buggy, we should always get at least a
>  		 * mapping to nobody */
> -		dprintk("RPC:       No creds found, marking Negative!\n");
> -		set_bit(CACHE_NEGATIVE, &rsci.h.flags);
> +		dprintk("RPC:       No creds found!\n");
> +		goto out;
>  	} else {
>  
>  		/* steal creds */

IIRC, we are doing this to avoid rapid upcall loops in the kernel, where
we keep hammering upcalls out and keep getting an error back.
I am not sure it is wise to not set a temp negative cache here.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall
  2013-10-10 15:37   ` Simo Sorce
@ 2013-10-10 19:23     ` J. Bruce Fields
  2013-10-10 20:45       ` Simo Sorce
  0 siblings, 1 reply; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-10 19:23 UTC (permalink / raw)
  To: Simo Sorce; +Cc: J. Bruce Fields, linux-nfs

On Thu, Oct 10, 2013 at 11:37:12AM -0400, Simo Sorce wrote:
> On Thu, 2013-10-10 at 11:15 -0400, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > For every other problem here we bail out with an error, but here for
> > some reason we're setting a negative cache entry (with, note, an
> > undefined expiry).
> > 
> > It seems simplest just to bail out in the same way as we do in other
> > cases.
> > 
> > Cc: Simo Sorce <simo@redhat.com>
> > Reported-by: Andi Kleen <andi@firstfloor.org>
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  net/sunrpc/auth_gss/svcauth_gss.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 09fb638..008cdad 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -1167,8 +1167,8 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> >  	if (!ud->found_creds) {
> >  		/* userspace seem buggy, we should always get at least a
> >  		 * mapping to nobody */
> > -		dprintk("RPC:       No creds found, marking Negative!\n");
> > -		set_bit(CACHE_NEGATIVE, &rsci.h.flags);
> > +		dprintk("RPC:       No creds found!\n");
> > +		goto out;
> >  	} else {
> >  
> >  		/* steal creds */
> 
> IIRC, we are doing this to avoid rapid upcall loops in the kernel, where
> we keep hammering upcalls out and keep getting an error back.

Looks like returning an error instead results in closing the connection
to the client, so, depends how the client replies I guess.

In any case I don't see why we'd treat this particular gss-proxy bug
differently than we would any other (like, say, passing down bad xdr, or
a gss context that we can't import).

--b.

> I am not sure it is wise to not set a temp negative cache here.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall
  2013-10-10 19:23     ` J. Bruce Fields
@ 2013-10-10 20:45       ` Simo Sorce
  2013-10-11 13:55         ` J. Bruce Fields
  0 siblings, 1 reply; 10+ messages in thread
From: Simo Sorce @ 2013-10-10 20:45 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: J. Bruce Fields, linux-nfs

On Thu, 2013-10-10 at 15:23 -0400, J. Bruce Fields wrote:
> On Thu, Oct 10, 2013 at 11:37:12AM -0400, Simo Sorce wrote:
> > On Thu, 2013-10-10 at 11:15 -0400, J. Bruce Fields wrote:
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > 
> > > For every other problem here we bail out with an error, but here for
> > > some reason we're setting a negative cache entry (with, note, an
> > > undefined expiry).
> > > 
> > > It seems simplest just to bail out in the same way as we do in other
> > > cases.
> > > 
> > > Cc: Simo Sorce <simo@redhat.com>
> > > Reported-by: Andi Kleen <andi@firstfloor.org>
> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > ---
> > >  net/sunrpc/auth_gss/svcauth_gss.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > > index 09fb638..008cdad 100644
> > > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > > @@ -1167,8 +1167,8 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> > >  	if (!ud->found_creds) {
> > >  		/* userspace seem buggy, we should always get at least a
> > >  		 * mapping to nobody */
> > > -		dprintk("RPC:       No creds found, marking Negative!\n");
> > > -		set_bit(CACHE_NEGATIVE, &rsci.h.flags);
> > > +		dprintk("RPC:       No creds found!\n");
> > > +		goto out;
> > >  	} else {
> > >  
> > >  		/* steal creds */
> > 
> > IIRC, we are doing this to avoid rapid upcall loops in the kernel, where
> > we keep hammering upcalls out and keep getting an error back.
> 
> Looks like returning an error instead results in closing the connection
> to the client, so, depends how the client replies I guess.
> 
> In any case I don't see why we'd treat this particular gss-proxy bug
> differently than we would any other (like, say, passing down bad xdr, or
> a gss context that we can't import).

Uhmm I did not recall that, I guess the change is ok then.

Simo.

> --b.
> 
> > I am not sure it is wise to not set a temp negative cache here.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Simo Sorce * Red Hat, Inc * New York


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall
  2013-10-10 20:45       ` Simo Sorce
@ 2013-10-11 13:55         ` J. Bruce Fields
  0 siblings, 0 replies; 10+ messages in thread
From: J. Bruce Fields @ 2013-10-11 13:55 UTC (permalink / raw)
  To: Simo Sorce; +Cc: J. Bruce Fields, linux-nfs

On Thu, Oct 10, 2013 at 04:45:52PM -0400, Simo Sorce wrote:
> On Thu, 2013-10-10 at 15:23 -0400, J. Bruce Fields wrote:
> > On Thu, Oct 10, 2013 at 11:37:12AM -0400, Simo Sorce wrote:
> > > On Thu, 2013-10-10 at 11:15 -0400, J. Bruce Fields wrote:
> > > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > > 
> > > > For every other problem here we bail out with an error, but here for
> > > > some reason we're setting a negative cache entry (with, note, an
> > > > undefined expiry).
> > > > 
> > > > It seems simplest just to bail out in the same way as we do in other
> > > > cases.
> > > > 
> > > > Cc: Simo Sorce <simo@redhat.com>
> > > > Reported-by: Andi Kleen <andi@firstfloor.org>
> > > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > > ---
> > > >  net/sunrpc/auth_gss/svcauth_gss.c |    4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > > > index 09fb638..008cdad 100644
> > > > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > > > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > > > @@ -1167,8 +1167,8 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> > > >  	if (!ud->found_creds) {
> > > >  		/* userspace seem buggy, we should always get at least a
> > > >  		 * mapping to nobody */
> > > > -		dprintk("RPC:       No creds found, marking Negative!\n");
> > > > -		set_bit(CACHE_NEGATIVE, &rsci.h.flags);
> > > > +		dprintk("RPC:       No creds found!\n");
> > > > +		goto out;
> > > >  	} else {
> > > >  
> > > >  		/* steal creds */
> > > 
> > > IIRC, we are doing this to avoid rapid upcall loops in the kernel, where
> > > we keep hammering upcalls out and keep getting an error back.
> > 
> > Looks like returning an error instead results in closing the connection
> > to the client, so, depends how the client replies I guess.
> > 
> > In any case I don't see why we'd treat this particular gss-proxy bug
> > differently than we would any other (like, say, passing down bad xdr, or
> > a gss context that we can't import).
> 
> Uhmm I did not recall that, I guess the change is ok then.

OK, thanks for the review.

--b.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-10-11 13:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-10 15:14 miscellaneous gss-proxy & krb5 fixes for 3.13 J. Bruce Fields
2013-10-10 15:15 ` [PATCH 1/4] svcrpc: fix gss-proxy NULL dereference in some error cases J. Bruce Fields
2013-10-10 15:35   ` Simo Sorce
2013-10-10 15:15 ` [PATCH 2/4] svcrpc: fix error-handling on badd gssproxy downcall J. Bruce Fields
2013-10-10 15:37   ` Simo Sorce
2013-10-10 19:23     ` J. Bruce Fields
2013-10-10 20:45       ` Simo Sorce
2013-10-11 13:55         ` J. Bruce Fields
2013-10-10 15:15 ` [PATCH 3/4] svcrpc: handle some gssproxy encoding errors J. Bruce Fields
2013-10-10 15:15 ` [PATCH 4/4] gss_krb5: document that we ignore sequence number J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).