On Sun, Aug 13, 2017 at 7:38 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Fri, 2017-08-11 at 15:12 +0000, Trond Myklebust wrote: >> On Fri, 2017-08-11 at 10:22 -0400, Jeff Layton wrote: >> > I think I wasn't clear here. I'm not proposing that you move everyone >> > to >> > KEYRING: credcaches. This would not be a visible change to userland. >> > We'd still use rpc.gssd to upcall for creds. >> > >> > What I'm saying is that instead of storing the creds in a hashtable >> > like >> > we do today, we'd just stash them in one of the keyrings hanging off >> > of >> > struct cred. >> > >> > Change all of the authgss_ops operations to do query/store from the >> > appropriate keyring directly. With that, the effective lifetime of >> > GSSAPI creds would be bounded by the lifetime of the keyrings that >> > hold >> > references to it. >> > >> > We'd probably need a new key_type for this to ensure that this >> > couldn't >> > be manipulated directly from userland. Or...maybe you'd still want to >> > allow userland to destroy the creds? No need for a new syscall with >> > that >> > -- they can just do a "keyctl unlink". There are a lot of options >> > here. >> > >> > It's a non-trivial amount of work though (rpcauth_lookupcred() on >> > down >> > would probably need to be reworked) and I haven't looked at it >> > detail. >> > Still, it seems like it could be a more modern and cleaner design >> > than >> > what we have today. >> > >> >> The main annoyance with going from a global to a local cache such as >> the keyrings is that it makes comparing credentials a lot more work. >> Today, because the credentials are essentially unique per server, we >> just do pointer comparisons. Once we have non-global caches, we would >> need to do more elaborate comparisons to ensure that the uid, gid, and >> list of groups match. >> That's also why we never made the leap to using 'struct cred', btw... > > > Ok, it does seem better to have a global cache from that standpoint. > Still, a new syscall for this doesn't seem very elegant. I also worry a > bit about writeback here too (like David and Neil have pointed out). > > What about changing how we hold references on these objects instead? > > After we look up an auth token in e.g. rpcauth_lookupcred, take a > reference to it and stash a pointer to it somewhere in the cred. > Possibly in the thread or process keyrings, but it may work better > elsewhere. > > When we go to look up creds from that thread in the future, we can get > to it directly (which is a nice bonus). When the cred is destroyed > (usually on process destruction), we'd drop the reference to the object, > which would drop the reference to the global cache object. > > The global cache could then be changed to have a pretty short timeout (a > few seconds?) and reap the object soon afterward when there are no more > active processes that have used it. Wouldn’t that produce a lot of unnecessary context re-establishments. > It's a bit more work and we might need to grow struct cred to handle it > (maybe give it its own keyring?), but it seems like that might be a > cleaner solution than giving userland knobs to manage the kernel's > caches. Userland is the only place that know that kdestroy ran and is the best place to tell the kernel to remove its cache. Everything else is guessing. > -- > Jeff Layton <jlayton@xxxxxxxxxx> > -- > 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
