| Message ID | 20220816193209.4057566-3-jackyli@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Improve error handling during INIT_EX file initialization | expand |
On Tue, 16 Aug 2022, Jacky Li wrote: > Currently the OS continues the PSP initialization when there is a write > failure to the init_ex_file. Therefore, the userspace would be told that > SEV is properly INIT'd even though the psp data file is not updated. > This is problematic because later when asked for the SEV data, the OS > won't be able to provide it. > > Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support") > Reported-by: Peter Gonda <pgonda@google.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Jacky Li <jackyli@google.com> Acked-by: David Rientjes <rientjes@google.com>
On 8/16/22 14:32, Jacky Li wrote: > Currently the OS continues the PSP initialization when there is a write > failure to the init_ex_file. Therefore, the userspace would be told that > SEV is properly INIT'd even though the psp data file is not updated. > This is problematic because later when asked for the SEV data, the OS > won't be able to provide it. > > Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support") > Reported-by: Peter Gonda <pgonda@google.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Jacky Li <jackyli@google.com> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > Changelog since v1: > - Add a blank line after the variable declaration. > - Fix the string format of the error code. > > drivers/crypto/ccp/sev-dev.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index fb7ca45a2f0d..ab1f76549ef8 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -237,7 +237,7 @@ static int sev_read_init_ex_file(void) > return 0; > } > > -static void sev_write_init_ex_file(void) > +static int sev_write_init_ex_file(void) > { > struct sev_device *sev = psp_master->sev_data; > struct file *fp; > @@ -247,14 +247,16 @@ static void sev_write_init_ex_file(void) > lockdep_assert_held(&sev_cmd_mutex); > > if (!sev_init_ex_buffer) > - return; > + return 0; > > fp = open_file_as_root(init_ex_path, O_CREAT | O_WRONLY, 0600); > if (IS_ERR(fp)) { > + int ret = PTR_ERR(fp); > + > dev_err(sev->dev, > - "SEV: could not open file for write, error %ld\n", > - PTR_ERR(fp)); > - return; > + "SEV: could not open file for write, error %d\n", > + ret); > + return ret; > } > > nwrite = kernel_write(fp, sev_init_ex_buffer, NV_LENGTH, &offset); > @@ -265,18 +267,20 @@ static void sev_write_init_ex_file(void) > dev_err(sev->dev, > "SEV: failed to write %u bytes to non volatile memory area, ret %ld\n", > NV_LENGTH, nwrite); > - return; > + return -EIO; > } > > dev_dbg(sev->dev, "SEV: write successful to NV file\n"); > + > + return 0; > } > > -static void sev_write_init_ex_file_if_required(int cmd_id) > +static int sev_write_init_ex_file_if_required(int cmd_id) > { > lockdep_assert_held(&sev_cmd_mutex); > > if (!sev_init_ex_buffer) > - return; > + return 0; > > /* > * Only a few platform commands modify the SPI/NV area, but none of the > @@ -291,10 +295,10 @@ static void sev_write_init_ex_file_if_required(int cmd_id) > case SEV_CMD_PEK_GEN: > break; > default: > - return; > + return 0; > } > > - sev_write_init_ex_file(); > + return sev_write_init_ex_file(); > } > > static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > @@ -367,7 +371,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > cmd, reg & PSP_CMDRESP_ERR_MASK); > ret = -EIO; > } else { > - sev_write_init_ex_file_if_required(cmd); > + ret = sev_write_init_ex_file_if_required(cmd); > } > > print_hex_dump_debug("(out): ", DUMP_PREFIX_OFFSET, 16, 2, data,
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index fb7ca45a2f0d..ab1f76549ef8 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -237,7 +237,7 @@ static int sev_read_init_ex_file(void) return 0; } -static void sev_write_init_ex_file(void) +static int sev_write_init_ex_file(void) { struct sev_device *sev = psp_master->sev_data; struct file *fp; @@ -247,14 +247,16 @@ static void sev_write_init_ex_file(void) lockdep_assert_held(&sev_cmd_mutex); if (!sev_init_ex_buffer) - return; + return 0; fp = open_file_as_root(init_ex_path, O_CREAT | O_WRONLY, 0600); if (IS_ERR(fp)) { + int ret = PTR_ERR(fp); + dev_err(sev->dev, - "SEV: could not open file for write, error %ld\n", - PTR_ERR(fp)); - return; + "SEV: could not open file for write, error %d\n", + ret); + return ret; } nwrite = kernel_write(fp, sev_init_ex_buffer, NV_LENGTH, &offset); @@ -265,18 +267,20 @@ static void sev_write_init_ex_file(void) dev_err(sev->dev, "SEV: failed to write %u bytes to non volatile memory area, ret %ld\n", NV_LENGTH, nwrite); - return; + return -EIO; } dev_dbg(sev->dev, "SEV: write successful to NV file\n"); + + return 0; } -static void sev_write_init_ex_file_if_required(int cmd_id) +static int sev_write_init_ex_file_if_required(int cmd_id) { lockdep_assert_held(&sev_cmd_mutex); if (!sev_init_ex_buffer) - return; + return 0; /* * Only a few platform commands modify the SPI/NV area, but none of the @@ -291,10 +295,10 @@ static void sev_write_init_ex_file_if_required(int cmd_id) case SEV_CMD_PEK_GEN: break; default: - return; + return 0; } - sev_write_init_ex_file(); + return sev_write_init_ex_file(); } static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) @@ -367,7 +371,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) cmd, reg & PSP_CMDRESP_ERR_MASK); ret = -EIO; } else { - sev_write_init_ex_file_if_required(cmd); + ret = sev_write_init_ex_file_if_required(cmd); } print_hex_dump_debug("(out): ", DUMP_PREFIX_OFFSET, 16, 2, data,
Currently the OS continues the PSP initialization when there is a write failure to the init_ex_file. Therefore, the userspace would be told that SEV is properly INIT'd even though the psp data file is not updated. This is problematic because later when asked for the SEV data, the OS won't be able to provide it. Fixes: 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support") Reported-by: Peter Gonda <pgonda@google.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Jacky Li <jackyli@google.com> --- Changelog since v1: - Add a blank line after the variable declaration. - Fix the string format of the error code. drivers/crypto/ccp/sev-dev.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)