On Wed, 2022-11-30 at 08:00 +0100, Hannes Reinecke wrote: > On 11/30/22 00:25, gjoyce@xxxxxxxxxxxxxxxxxx wrote: > > From: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx> > > > > Extend the SED block driver so it can alternatively > > obtain a key from a sed-opal kernel keyring. The SED > > ioctls will indicate the source of the key, either > > directly in the ioctl data or from the keyring. > > > > This allows the use of SED commands in scripts such as > > udev scripts so that drives may be automatically unlocked > > as they become available. > > > > Signed-off-by: Greg Joyce <gjoyce@xxxxxxxxxxxxxxxxxx> > > Reviewed-by: Jonathan Derrick <jonathan.derrick@xxxxxxxxx> > > --- > > block/Kconfig | 1 + > > block/sed-opal.c | 174 > > +++++++++++++++++++++++++++++++++- > > include/linux/sed-opal.h | 3 + > > include/uapi/linux/sed-opal.h | 8 +- > > 4 files changed, 183 insertions(+), 3 deletions(-) > > > > + ret = opal_get_key(dev, &opal_lrs->session.opal_key); > > + if (ret) > > + return ret; > > mutex_lock(&dev->dev_lock); > > setup_opal_dev(dev); > > ret = execute_steps(dev, lr_steps, ARRAY_SIZE(lr_steps)); > > @@ -2622,6 +2759,14 @@ static int opal_set_new_pw(struct opal_dev > > *dev, struct opal_new_pw *opal_pw) > > ret = execute_steps(dev, pw_steps, ARRAY_SIZE(pw_steps)); > > mutex_unlock(&dev->dev_lock); > > > > + if (ret) > > + return ret; > > + > > + /* update keyring with new password */ > > + ret = update_sed_opal_key(OPAL_AUTH_KEY, > > + opal_pw->new_user_pw.opal_key.key, > > + opal_pw- > > >new_user_pw.opal_key.key_len); > > + > > return ret; > > } > > > What about key revocation? > You only allow to set a new key, but what happens with the old ones? My understanding was that key_create_or_update() would not allow duplicates so there shouldn't be old ones. Is that incorrect? > > > +static int __init sed_opal_init(void) > > +{ > > + struct key *kr; > > + > > + kr = keyring_alloc(".sed_opal", > > + GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, > > current_cred(), > > + (KEY_POS_ALL & ~KEY_POS_SETATTR) | > > KEY_USR_VIEW | > > + KEY_USR_READ | KEY_USR_SEARCH | > > KEY_USR_WRITE, > > + KEY_ALLOC_NOT_IN_QUOTA, > > + NULL, NULL); > > + if (IS_ERR(kr)) > > + return PTR_ERR(kr); > > + > > + sed_opal_keyring = kr; > > + > > + return 0; > > +} > > +late_initcall(sed_opal_init); > > Shouldn't you free the keyring on exit? The SED Opal driver is part of the block driver and does not build as a module so it will not exit. I had looked at "blacklist" as an example and saw that it allocated but did not free its keyring. > Cheers, > > Hannes Thanks for the comments on the keyring. I'm not very familiar with the keyring code, so I'd appreciate suggestions on code changes if any are needed for your two comments. -Greg