> -----Original Message----- > From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs- > owner@xxxxxxxxxxxxxxx] On Behalf Of Chuck Lever > Sent: Thursday, January 24, 2013 11:14 AM > To: NeilBrown > Cc: Kevin Coffman; J.Bruce Fields; Steve Dickson; NFS > Subject: Re: Corrupted RPC_GSS_PROC_DESTROY packets coming from Linux > servers. > > > On Jan 23, 2013, at 9:02 PM, NeilBrown <neilb@xxxxxxx> wrote: > > > > > Hi peoples, > > > > this issue has appeared on the mailing list before (particularly > > around July > > 2011) but hasn't been resolved yet and it just bit me again so I > > figure it is time it got fixed. > > > > > > If you tcpdump the network connection while mounting an NFS filesystem > > using kerberos - or while the client is establishing a new context > > because e.g. the server rebooted - you will see a NULL RPC with an > > RPC_GSS_PROC_DESTROY credential but no verifier. The lack of a > > verifier makes the packet corrupt so the server ignores it, but people > > see it and think something is wrong. > > > > It is good that the server ignores it as it really shouldn't be there. > > What happens is that the NFS client calls up to rpc.gssd to request a > > credential. rpc.gssd then establishes a connection directly with the > > server, including the establishment of the security context. Then it > > gathers the context details and passed them down to the kernel. > > Then it closes the connection part of which involves calling > > AUTH_DESTROY(auth) - necessary to free up data structures and not leak > > memory. > > This AUTH_DESTROY tries to destroy the context completely, including > > telling the server that it has been destroyed! But it hasn't, it has > > just been passed down to the kernel for use on a different connection. > > > > So there are two issues here: > > - why is the GSS_PROC_DESTROY packet missing a verifier > > - how can we get AUTH_DESTROY to *not* try to destroy the context on > the > > server - as that would be a bad thing. > > > > The first I cannot completely answer. I do know that in libtirpc, in > > auth_gss.c, in authgss_marshal(), gss_get_mic is failing because it doesn't > > think it has a valid context. I don't know why it thinks that, and I don't > > really care. > > > > > > The second question is more interesting and I see two possible options. > > > > 1/ If we knew why gss_get_mic failed and had good reason to believe it > > would keep on failing, we could consider changing clnt_vc_call to > > respond to an error from AUTH_MARSHALL not by sending a truncated > > packet, but by purging the current message and not sending it at all. > > This should be possible but might be messy. > > > > 2/ Make libtirpc behave more like librpcsecgss. > > In libtirpc, the authgss_get_private_data() function just hands over > > a pointer to the private data, but keeps its own pointer so it can > > free it when the client is finally destroyed. > > > > In librpcsecgss, since commit > > 07fce317cac267509b944a8191cafa8e49b5e328 > > (thanks Kevin), authgss_get_private_data() hands the data over to the > > caller and doesn't keep it's own reference to it. So the caller has > > to call > > authgss_free_private_data() when it has finished with the data. > > As the library no longer has the credential, it doesn't even bother > > trying to send a GSS_PROC_DESTROY request. > > > > When Chuck noticed this difference between the two libraries, he > > resolved it - in commit 336f8bca825416082d62ef38314f3e0b7e8f5cc2 as > follow: > > > > if (token.value) > > free(token.value); > > +#ifndef HAVE_LIBTIRPC > > if (pd.pd_ctx_hndl.length != 0) > > authgss_free_private_data(&pd); > > +#endif > > > > Clearly to significance of this difference was not obvious, and this > > was the easiest fix. > > > > If we were to "fix" this properly, we would need to add a commit like > > the one from Kevin to libtirpc, and remove that #ifndef from nfs-utils. > > co-ordinating this might be tricking. nfs-utils could presumably > > test if libtirpc provided the function (at configure time) and call > > it if it does, > > This seems to me like the best approach for 2. > > > However is someone updates libtirpc without updating or recompiling > > nfs-utils they would get a memory leak. May it would be slow enough > > not to be serious, and if anyone noticed that could just upgrade and get a > fix. > > Telling people to upgrade for a fix is what we do for a living. In all > seriousness, though, in the common case, people will be using nfs-utils and > libtirpc built by distributions, and we expect the distros will get the fix > dependency right over time. > > > Does this seem reasonable? How is maintaining libtirpc these days? > > Could we get the fix into 0.2.3, or would we need a minor version > > bump to 0.3.0?? > > A minor version bump shouldn't be necessary if we're not changing the > synopsis of a published API, nor are we removing a published API. > > > 3/ there is actually a third option. We could change > > authgss_get_private_data() to set gc.gc_ctx.length to 0, but not free > > the buffer. Then aithgss_destroy_context() could notice that the > > length is zero and the buffer is not NULL, and could free the buffer but not > try to send > > the context_destroy request. It's an ugly hack though and I think I'd > > rather not. 4/ Have authgss_get_private_data() consume the 'auth' argument. Reusing the auth in an RPC call after we've transferred the context to the kernel is in any case a bug, so why allow it at all? Cheers, 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
