On Tue, 2022-12-20 at 18:16 +1100, Andrew Donnellan wrote: > From: Nayna Jain <na...@linux.ibm.com> > > The Platform Keystore provides a signed update interface which can be > used > to create, replace or append to certain variables in the PKS in a > secure > fashion, with the hypervisor requiring that the update be signed > using the > Platform Key. > > Implement an interface to the H_PKS_SIGNED_UPDATE hcall in the plpks > driver to allow signed updates to PKS objects. > > (The plpks driver doesn't need to do any cryptography or otherwise > handle > the actual signed variable contents - that will be handled by > userspace > tooling.) > > Signed-off-by: Nayna Jain <na...@linux.ibm.com> > [ajd: split patch, rewrite commit message, add timeout handling] > Co-developed-by: Andrew Donnellan <a...@linux.ibm.com> > Signed-off-by: Andrew Donnellan <a...@linux.ibm.com> > --- > arch/powerpc/include/asm/hvcall.h | 3 +- > arch/powerpc/platforms/pseries/plpks.c | 81 +++++++++++++++++++++++- > -- > arch/powerpc/platforms/pseries/plpks.h | 5 ++ > 3 files changed, 79 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/include/asm/hvcall.h > b/arch/powerpc/include/asm/hvcall.h > index 95fd7f9485d5..33b26c0cb69b 100644 > --- a/arch/powerpc/include/asm/hvcall.h > +++ b/arch/powerpc/include/asm/hvcall.h > @@ -336,7 +336,8 @@ > #define H_SCM_FLUSH 0x44C > #define H_GET_ENERGY_SCALE_INFO 0x450 > #define H_WATCHDOG 0x45C > -#define MAX_HCALL_OPCODE H_WATCHDOG > +#define H_PKS_SIGNED_UPDATE 0x454 > +#define MAX_HCALL_OPCODE H_PKS_SIGNED_UPDATE > > /* Scope args for H_SCM_UNBIND_ALL */ > #define H_UNBIND_SCOPE_ALL (0x1) > diff --git a/arch/powerpc/platforms/pseries/plpks.c > b/arch/powerpc/platforms/pseries/plpks.c > index c5ae00a8a968..9e4401aabf4f 100644 > --- a/arch/powerpc/platforms/pseries/plpks.c > +++ b/arch/powerpc/platforms/pseries/plpks.c > @@ -30,9 +30,9 @@ > #define MAX_NAME_SIZE 239 > #define MAX_DATA_SIZE 4000 > > -#define PKS_FLUSH_MAX_TIMEOUT 5000 //msec > -#define PKS_FLUSH_SLEEP 10 //msec > -#define PKS_FLUSH_SLEEP_RANGE 400 > +#define PKS_MAX_TIMEOUT 5000 // msec > +#define PKS_FLUSH_SLEEP 10 // msec > +#define PKS_FLUSH_SLEEP_RANGE 400 > > static u8 *ospassword; > static u16 ospasswordlength; > @@ -95,6 +95,12 @@ static int pseries_status_to_err(int rc) > err = -ENOENT; > break; > case H_BUSY: > + case H_LONG_BUSY_ORDER_1_MSEC: > + case H_LONG_BUSY_ORDER_10_MSEC: > + case H_LONG_BUSY_ORDER_100_MSEC: > + case H_LONG_BUSY_ORDER_1_SEC: > + case H_LONG_BUSY_ORDER_10_SEC: > + case H_LONG_BUSY_ORDER_100_SEC: > err = -EBUSY; > break; > case H_AUTHORITY: > @@ -198,14 +204,17 @@ static struct label *construct_label(char > *component, u8 varos, u8 *name, > u16 namelen) > { > struct label *label; > - size_t slen; > + size_t slen = 0; > > if (!name || namelen > MAX_NAME_SIZE) > return ERR_PTR(-EINVAL); > > - slen = strlen(component); > - if (component && slen > sizeof(label->attr.prefix)) > - return ERR_PTR(-EINVAL); > + // Support NULL component for signed updates > + if (component) { > + slen = strlen(component); > + if (slen > sizeof(label->attr.prefix)) > + return ERR_PTR(-EINVAL); > + } > > // The label structure must not cross a page boundary, so we > align to the next power of 2 > label = kzalloc(roundup_pow_of_two(sizeof(*label)), > GFP_KERNEL); > @@ -372,7 +381,7 @@ static int plpks_confirm_object_flushed(struct > label *label, > usleep_range(PKS_FLUSH_SLEEP, > PKS_FLUSH_SLEEP + > PKS_FLUSH_SLEEP_RANGE); > timeout = timeout + PKS_FLUSH_SLEEP; > - } while (timeout < PKS_FLUSH_MAX_TIMEOUT); > + } while (timeout < PKS_MAX_TIMEOUT); > > if (timed_out) > rc = -ETIMEDOUT; > @@ -382,6 +391,60 @@ static int plpks_confirm_object_flushed(struct > label *label, > return rc; > } > > +int plpks_signed_update_var(struct plpks_var var, u64 flags) > +{ > + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = {0}; > + int rc; > + struct label *label; > + struct plpks_auth *auth; > + u64 continuetoken = 0; > + u64 timeout = 0; > + > + if (!var.data || var.datalen <= 0 || var.namelen > > MAX_NAME_SIZE) > + return -EINVAL; > + > + if (!(var.policy & SIGNEDUPDATE)) > + return -EINVAL; > + > + auth = construct_auth(PKS_OS_OWNER); > + if (IS_ERR(auth)) > + return PTR_ERR(auth); > + > + label = construct_label(var.component, var.os, var.name, > var.namelen); > + if (IS_ERR(label)) { > + rc = PTR_ERR(label); > + goto out; > + } > + > + do { > + rc = plpar_hcall9(H_PKS_SIGNED_UPDATE, retbuf, > + virt_to_phys(auth), > virt_to_phys(label), > + label->size, var.policy, flags, > + virt_to_phys(var.data), > var.datalen, > + continuetoken); > + > + continuetoken = retbuf[0]; > + if (pseries_status_to_err(rc) == -EBUSY) { > + int delay_ms = get_longbusy_msecs(rc); > + mdelay(delay_ms); > + timeout += delay_ms; > + } > + rc = pseries_status_to_err(rc); > + } while (rc == -EBUSY && timeout < PKS_MAX_TIMEOUT); > + > + if (!rc) { > + rc = plpks_confirm_object_flushed(label, auth); > + rc = pseries_status_to_err(rc);
Doesn't plpks_confirm_object_flushed() already return a Linux-friendly error code? If I'm reading this right, we'd be replacing any non-zero return code with -EINVAL. > + } > + > + kfree(label); > +out: > + kfree(auth); > + > + return rc; > +} > +EXPORT_SYMBOL(plpks_signed_update_var); > + > int plpks_write_var(struct plpks_var var) > { > unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 }; > @@ -428,7 +491,7 @@ int plpks_remove_var(char *component, u8 varos, > struct plpks_var_name vname) > struct label *label; > int rc; > > - if (!component || vname.namelen > MAX_NAME_SIZE) > + if (vname.namelen > MAX_NAME_SIZE) > return -EINVAL; > > auth = construct_auth(PKS_OS_OWNER); > diff --git a/arch/powerpc/platforms/pseries/plpks.h > b/arch/powerpc/platforms/pseries/plpks.h > index c89740796660..33cf12809392 100644 > --- a/arch/powerpc/platforms/pseries/plpks.h > +++ b/arch/powerpc/platforms/pseries/plpks.h > @@ -40,6 +40,11 @@ struct plpks_var_name_list { > struct plpks_var_name varlist[]; > }; > > +/** > + * Updates the authenticated variable. It expects NULL as the > component. > + */ > +int plpks_signed_update_var(struct plpks_var var, u64 flags); > + > /** > * Writes the specified var and its data to PKS. > * Any caller of PKS driver should present a valid component type > for