Re: upstream nfsd kernel oops in auth_rpcgss

["J. Bruce Fields" <bfields@redhat.com>]

On Mon, Jan 30, 2017 at 04:02:37PM -0500, J. Bruce Fields wrote:
> On Mon, Jan 30, 2017 at 03:15:36PM -0500, Olga Kornievskaia wrote:
> > Hi Bruce,
> > 
> > I ran into this oops in the nfsd (below) (4.10-rc3 kernel). To trigger this I had a client (unsuccessfully) try to mount the server with krb5 where the server doesn’t have the rpcsec_gss_krb5 module built. 
> 
> Whoops, thanks for catching that.
> 
> > Below code seems to fix things:
> > 
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 886e9d38..7faffd8 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -1187,9 +1187,9 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> >  		status = -EOPNOTSUPP;
> >  		/* get mech handle from OID */
> >  		gm = gss_mech_get_by_OID(&ud->mech_oid);
> > +		rsci.cred.cr_gss_mech = gm;
> >  		if (!gm)
> >  			goto out;
> > -		rsci.cred.cr_gss_mech = gm;
> >  
> >  		status = -EINVAL;
> >  		/* mech-specific data: */
> > 
> > I guess the problem is because rsci.cred gets initialed from something that’s passed in that doesn’t have it’s cr_gss_mech initialized to NULL. So when gss_mech_get_by_OID() fails and it calls rsc_free() and tries to free a bad pointer. 
> 
> Hm, yes, just above there's:
> 
> 	rsci.cred = ud->creds;
> 
> and it does make me a little nervous if ud->creds isn't completely
> initialized.
> 
> The only caller of gss_proxy_save_rsc is svcauth_gss_proxy_init:
> 
> 	status = gss_proxy_save_rsc(sn->rsc_cache, &ud, &handle);
> 
> ud there is on the stack, and initialized by:
> 
> 	memset(&ud, 0, sizeof(ud));
> 
> Hm, in gssp_accept_sec_context_upcall, there's:
> 
> 	data->creds = *(struct svc_cred *)value->data;
> 
> That looks really weird.  I'm not sure what it should be.

Does this do it?

--b.

diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index dc6fb79a361f..25d9a9cf7b66 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -260,7 +260,7 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,
 	if (!oa->data)
 		return -ENOMEM;
 
-	creds = kmalloc(sizeof(struct svc_cred), GFP_KERNEL);
+	creds = kzalloc(sizeof(struct svc_cred), GFP_KERNEL);
 	if (!creds) {
 		kfree(oa->data);
 		return -ENOMEM;