All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.