On Tue, 17 Jul 2012 21:46:39 +0100 David Howells <[email protected]> wrote:
> Jeff Layton <[email protected]> wrote: > > > +static void > > +check_session_keyring(void) > > +{ > > + key_serial_t ses_key, uses_key; > > + > > + ses_key = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0); > > + if (ses_key == -1) > > + return; > > + > > + uses_key = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0); > > + if (uses_key == -1) > > + return; > > + > > + if (ses_key == uses_key) > > + fprintf(stderr, "Warning: you have no session keyring. " > > + "cifscreds keys will not persist. See " > > + "pam_keyinit(8).\n"); > > +} > > I would suggest reporting an error and exiting in the event that certainly the > first call returns -1 and maybe the second. > Yeah, I considered that. I figured the follow-on keyctl() calls would end up erroring out, but I suppose doing it at this point would be more efficient. I'll respin the patch in a bit... > Other than that, it looks okay. > > I wonder if I should suggest giving an error if you try and modify the session > keyring when there isn't one (where modification includes adding a key to it). > When I first did the keyring stuff in the kernel, I didn't envision > pam_keyinit > - which in retrospect is a much better way of generating the session keyring > than having the kernel try to guess. > > Note that this would not stop processes joining a session keyring or creating > a > new session keyring. > That might be more reasonable. Spawning a new session keyring on the fly like is done today seems to be of questionable value. Thanks, -- Jeff Layton <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
