From: Greg Joyce <gjoyce@linux.vnet.ibm.com> To: Hannes Reinecke <hare@suse.de>, linux-block@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org, jonathan.derrick@linux.dev, brking@linux.vnet.ibm.com, msuchanek@suse.de, mpe@ellerman.id.au, nayna@linux.ibm.com, axboe@kernel.dk, akpm@linux-foundation.org, keyrings@vger.kernel.org Subject: Re: [PATCH v3 3/3] block: sed-opal: keyring support for SED keys Date: Thu, 01 Dec 2022 12:03:57 -0600 [thread overview] Message-ID: <ed32cbc546383085bc8c00d913a53059831b2cfc.camel@linux.vnet.ibm.com> (raw) In-Reply-To: <c78edd60-b6ae-6ec0-9ce4-73b9a92b9b32@suse.de> On Wed, 2022-11-30 at 08:00 +0100, Hannes Reinecke wrote: > On 11/30/22 00:25, gjoyce@linux.vnet.ibm.com wrote: > > From: Greg Joyce <gjoyce@linux.vnet.ibm.com> > > > > 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@linux.vnet.ibm.com> > > Reviewed-by: Jonathan Derrick <jonathan.derrick@linux.dev> > > --- > > 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
WARNING: multiple messages have this Message-ID (diff)
From: Greg Joyce <gjoyce@linux.vnet.ibm.com> To: Hannes Reinecke <hare@suse.de>, linux-block@vger.kernel.org Cc: axboe@kernel.dk, nayna@linux.ibm.com, keyrings@vger.kernel.org, jonathan.derrick@linux.dev, brking@linux.vnet.ibm.com, akpm@linux-foundation.org, msuchanek@suse.de, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v3 3/3] block: sed-opal: keyring support for SED keys Date: Thu, 01 Dec 2022 12:03:57 -0600 [thread overview] Message-ID: <ed32cbc546383085bc8c00d913a53059831b2cfc.camel@linux.vnet.ibm.com> (raw) In-Reply-To: <c78edd60-b6ae-6ec0-9ce4-73b9a92b9b32@suse.de> On Wed, 2022-11-30 at 08:00 +0100, Hannes Reinecke wrote: > On 11/30/22 00:25, gjoyce@linux.vnet.ibm.com wrote: > > From: Greg Joyce <gjoyce@linux.vnet.ibm.com> > > > > 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@linux.vnet.ibm.com> > > Reviewed-by: Jonathan Derrick <jonathan.derrick@linux.dev> > > --- > > 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
next prev parent reply other threads:[~2022-12-01 18:04 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-29 23:25 [PATCH v3 0/3] sed-opal: keyrings, discovery, revert, key store gjoyce 2022-11-29 23:25 ` gjoyce 2022-11-29 23:25 ` [PATCH v3 1/3] block: sed-opal: Implement IOC_OPAL_DISCOVERY gjoyce 2022-11-29 23:25 ` gjoyce 2022-11-30 6:52 ` Hannes Reinecke 2022-11-30 6:52 ` Hannes Reinecke 2022-11-29 23:25 ` [PATCH v3 2/3] block: sed-opal: Implement IOC_OPAL_REVERT_LSP gjoyce 2022-11-29 23:25 ` gjoyce 2022-11-30 6:53 ` Hannes Reinecke 2022-11-30 6:53 ` Hannes Reinecke 2022-11-29 23:25 ` [PATCH v3 3/3] block: sed-opal: keyring support for SED keys gjoyce 2022-11-29 23:25 ` gjoyce 2022-11-30 7:00 ` Hannes Reinecke 2022-11-30 7:00 ` Hannes Reinecke 2022-11-30 15:19 ` Greg Joyce 2022-11-30 15:19 ` Greg Joyce 2022-12-01 3:46 ` Ben Boeckel 2022-12-01 3:46 ` Ben Boeckel 2022-12-01 15:29 ` Greg Joyce 2022-12-01 15:29 ` Greg Joyce 2022-12-01 16:12 ` Ben Boeckel 2022-12-01 16:12 ` Ben Boeckel 2022-12-01 16:58 ` Greg Joyce 2022-12-01 16:58 ` Greg Joyce 2022-12-01 17:00 ` Greg Joyce 2022-12-01 17:00 ` Greg Joyce 2022-12-01 18:03 ` Greg Joyce [this message] 2022-12-01 18:03 ` Greg Joyce 2022-12-02 6:56 ` Hannes Reinecke 2022-12-02 6:56 ` Hannes Reinecke 2022-12-02 15:18 ` Greg Joyce 2022-12-02 15:18 ` Greg Joyce 2022-12-04 0:05 kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ed32cbc546383085bc8c00d913a53059831b2cfc.camel@linux.vnet.ibm.com \ --to=gjoyce@linux.vnet.ibm.com \ --cc=akpm@linux-foundation.org \ --cc=axboe@kernel.dk \ --cc=brking@linux.vnet.ibm.com \ --cc=hare@suse.de \ --cc=jonathan.derrick@linux.dev \ --cc=keyrings@vger.kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=msuchanek@suse.de \ --cc=nayna@linux.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.