On Tue, 17 Jul 2012 21:46:39 +0100
David Howells <dhowells@xxxxxxxxxx> wrote:
> Jeff Layton <jlayton@xxxxxxxxx> 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 <jlayton@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
