Re: [PATCH v4 3/3] block: sed-opal: keystore access for SED Opal keys

[Greg Joyce <gjoyce@linux.vnet.ibm.com>]

On Fri, 2022-10-07 at 12:21 -0600, Jonathan Derrick wrote:
> LGTM besides comment below
> 
> Reviewed-by: Jonathan Derrick <jonathan.derrick@linux.dev>
> 
> On 8/19/2022 4:31 PM, gjoyce@linux.vnet.ibm.com wrote:
> > From: Greg Joyce <gjoyce@linux.vnet.ibm.com>
> > 
> > Allow for permanent SED authentication keys by
> > reading/writing to the SED Opal non-volatile keystore.
> > 
> > Signed-off-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>
> > ---
> >  block/sed-opal.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/sed-opal.c b/block/sed-opal.c
> > index 3bdb31cf3e7c..11b0eb3a656b 100644
> > --- a/block/sed-opal.c
> > +++ b/block/sed-opal.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/uaccess.h>
> >  #include <uapi/linux/sed-opal.h>
> >  #include <linux/sed-opal.h>
> > +#include <linux/sed-opal-key.h>
> >  #include <linux/string.h>
> >  #include <linux/kdev_t.h>
> >  #include <linux/key.h>
> > @@ -2697,7 +2698,13 @@ static int opal_set_new_pw(struct opal_dev
> > *dev, struct opal_new_pw *opal_pw)
> >  	if (ret)
> >  		return ret;
> >  
> > -	/* update keyring with new password */
> > +	/* update keyring and arch var with new password */
> > +	ret = sed_write_key(OPAL_AUTH_KEY,
> > +			    opal_pw->new_user_pw.opal_key.key,
> > +			    opal_pw->new_user_pw.opal_key.key_len);
> > +	if (ret != -EOPNOTSUPP)
> > +		pr_warn("error updating SED key: %d\n", ret);
> I cant see any reason this would fail and make the keys inconsistent,
> but it seems
> like update_sed_opal_key() should be dependent on sed_write_key()
> succeeding

The thought was that since the key was already updated on the SED
drive, there should be an attempt to update it in the key store
even in the unlikely event the keyring update failed.

> 
> > +
> >  	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);
> > @@ -2920,6 +2927,8 @@ EXPORT_SYMBOL_GPL(sed_ioctl);
> >  static int __init sed_opal_init(void)
> >  {
> >  	struct key *kr;
> > +	char init_sed_key[OPAL_KEY_MAX];
> > +	int keylen = OPAL_KEY_MAX;
> >  
> >  	kr = keyring_alloc(".sed_opal",
> >  			   GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> > current_cred(),
> > @@ -2932,6 +2941,11 @@ static int __init sed_opal_init(void)
> >  
> >  	sed_opal_keyring = kr;
> >  
> > -	return 0;
> > +	if (sed_read_key(OPAL_AUTH_KEY, init_sed_key, &keylen) < 0) {
> > +		memset(init_sed_key, '\0', sizeof(init_sed_key));
> > +		keylen = OPAL_KEY_MAX;
> > +	}
> > +
> > +	return update_sed_opal_key(OPAL_AUTH_KEY, init_sed_key,
> > keylen);
> >  }
> >  late_initcall(sed_opal_init);