On Mon, Jan 30, 2017 at 4:23 PM, J. Bruce Fields <bfields@xxxxxxxxxx> wrote:
> 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;
Yes, it does!
> --
> 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
--
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
