| Message ID | 20241016145708.1166471-2-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Resolve potential blocking-forever issue | expand |
On Wed, Oct 16, 2024 at 10:57:07AM -0400, Stefan Berger wrote: > Use the new ptm_cap_n structure for getting the PTM_GET_CAPABILITY response > from swtpm. Previously only 17 bits could possibly have been set in ptm_cap > (=uint64_t) in big endian order and those bits are now found in the 2nd > 32bit word in the response in the caps field. > > This data structure makes it now clear that the 1st 32bit word carries the > tpm_result like all the other response structures of all other commands > do. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > backends/tpm/tpm_emulator.c | 14 ++++++++------ > backends/tpm/tpm_ioctl.h | 13 ++++++++++++- > backends/tpm/trace-events | 2 +- > 3 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h > index 1933ab6855..ee2dd15d35 100644 > --- a/backends/tpm/tpm_ioctl.h > +++ b/backends/tpm/tpm_ioctl.h > @@ -29,6 +29,16 @@ > > typedef uint32_t ptm_res; > > +/* PTM_GET_CAPABILITY: Get supported capabilities (ioctl's) */ > +struct ptm_cap_n { > + union { > + struct { > + ptm_res tpm_result; /* will always be TPM_SUCCESS (0) */ > + uint32_t caps; > + } resp; /* response */ > + } u; > +}; The union here is pointless surely, since it only has one entry and the following patch doesn't add a 2nd either ? With regards, Daniel
On 10/16/24 11:03 AM, Daniel P. Berrangé wrote: > On Wed, Oct 16, 2024 at 10:57:07AM -0400, Stefan Berger wrote: >> Use the new ptm_cap_n structure for getting the PTM_GET_CAPABILITY response >> from swtpm. Previously only 17 bits could possibly have been set in ptm_cap >> (=uint64_t) in big endian order and those bits are now found in the 2nd >> 32bit word in the response in the caps field. >> >> This data structure makes it now clear that the 1st 32bit word carries the >> tpm_result like all the other response structures of all other commands >> do. >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> backends/tpm/tpm_emulator.c | 14 ++++++++------ >> backends/tpm/tpm_ioctl.h | 13 ++++++++++++- >> backends/tpm/trace-events | 2 +- >> 3 files changed, 21 insertions(+), 8 deletions(-) >> > >> diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h >> index 1933ab6855..ee2dd15d35 100644 >> --- a/backends/tpm/tpm_ioctl.h >> +++ b/backends/tpm/tpm_ioctl.h >> @@ -29,6 +29,16 @@ >> >> typedef uint32_t ptm_res; >> >> +/* PTM_GET_CAPABILITY: Get supported capabilities (ioctl's) */ >> +struct ptm_cap_n { >> + union { >> + struct { >> + ptm_res tpm_result; /* will always be TPM_SUCCESS (0) */ >> + uint32_t caps; >> + } resp; /* response */ >> + } u; >> +}; > > The union here is pointless surely, since it only has one entry and > the following patch doesn't add a 2nd either ? I did this because all commands have a union. > > > With regards, > Daniel
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c index 5a8fba9bde..b0e2fb3fc7 100644 --- a/backends/tpm/tpm_emulator.c +++ b/backends/tpm/tpm_emulator.c @@ -72,7 +72,7 @@ struct TPMEmulator { CharBackend ctrl_chr; QIOChannel *data_ioc; TPMVersion tpm_version; - ptm_cap caps; /* capabilities of the TPM */ + uint32_t caps; /* capabilities of the TPM */ uint8_t cur_locty_number; /* last set locality */ Error *migration_blocker; @@ -239,13 +239,15 @@ static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd, static int tpm_emulator_probe_caps(TPMEmulator *tpm_emu) { + ptm_cap_n cap_n; + if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_CAPABILITY, - &tpm_emu->caps, 0, sizeof(tpm_emu->caps)) < 0) { + &cap_n, 0, sizeof(cap_n)) < 0) { error_report("tpm-emulator: probing failed : %s", strerror(errno)); return -1; } - tpm_emu->caps = be64_to_cpu(tpm_emu->caps); + tpm_emu->caps = be32_to_cpu(cap_n.u.resp.caps); trace_tpm_emulator_probe_caps(tpm_emu->caps); @@ -254,7 +256,7 @@ static int tpm_emulator_probe_caps(TPMEmulator *tpm_emu) static int tpm_emulator_check_caps(TPMEmulator *tpm_emu) { - ptm_cap caps = 0; + uint32_t caps = 0; const char *tpm = NULL; /* check for min. required capabilities */ @@ -527,8 +529,8 @@ static size_t tpm_emulator_get_buffer_size(TPMBackend *tb) static int tpm_emulator_block_migration(TPMEmulator *tpm_emu) { Error *err = NULL; - ptm_cap caps = PTM_CAP_GET_STATEBLOB | PTM_CAP_SET_STATEBLOB | - PTM_CAP_STOP; + uint32_t caps = PTM_CAP_GET_STATEBLOB | PTM_CAP_SET_STATEBLOB | + PTM_CAP_STOP; if (!TPM_EMULATOR_IMPLEMENTS_ALL_CAPS(tpm_emu, caps)) { error_setg(&tpm_emu->migration_blocker, diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h index 1933ab6855..ee2dd15d35 100644 --- a/backends/tpm/tpm_ioctl.h +++ b/backends/tpm/tpm_ioctl.h @@ -29,6 +29,16 @@ typedef uint32_t ptm_res; +/* PTM_GET_CAPABILITY: Get supported capabilities (ioctl's) */ +struct ptm_cap_n { + union { + struct { + ptm_res tpm_result; /* will always be TPM_SUCCESS (0) */ + uint32_t caps; + } resp; /* response */ + } u; +}; + /* PTM_GET_TPMESTABLISHED: get the establishment bit */ struct ptm_est { union { @@ -242,7 +252,8 @@ struct ptm_lockstorage { } u; }; -typedef uint64_t ptm_cap; +typedef uint64_t ptm_cap; /* CUSE-only; use ptm_cap_n otherwise */ +typedef struct ptm_cap_n ptm_cap_n; typedef struct ptm_est ptm_est; typedef struct ptm_reset_est ptm_reset_est; typedef struct ptm_loc ptm_loc; diff --git a/backends/tpm/trace-events b/backends/tpm/trace-events index cb5cfa6510..05e30533ce 100644 --- a/backends/tpm/trace-events +++ b/backends/tpm/trace-events @@ -16,7 +16,7 @@ tpm_util_show_buffer_content(const char *buf) "%s" # tpm_emulator.c tpm_emulator_set_locality(uint8_t locty) "setting locality to %d" tpm_emulator_handle_request(void) "processing TPM command" -tpm_emulator_probe_caps(uint64_t caps) "capabilities: 0x%"PRIx64 +tpm_emulator_probe_caps(uint32_t caps) "capabilities: 0x%x" tpm_emulator_set_buffer_size(uint32_t buffersize, uint32_t minsize, uint32_t maxsize) "buffer size: %u, min: %u, max: %u" tpm_emulator_startup_tpm_resume(bool is_resume, size_t buffersize) "is_resume: %d, buffer size: %zu" tpm_emulator_get_tpm_established_flag(uint8_t flag) "got established flag: %d"
Use the new ptm_cap_n structure for getting the PTM_GET_CAPABILITY response from swtpm. Previously only 17 bits could possibly have been set in ptm_cap (=uint64_t) in big endian order and those bits are now found in the 2nd 32bit word in the response in the caps field. This data structure makes it now clear that the 1st 32bit word carries the tpm_result like all the other response structures of all other commands do. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- backends/tpm/tpm_emulator.c | 14 ++++++++------ backends/tpm/tpm_ioctl.h | 13 ++++++++++++- backends/tpm/trace-events | 2 +- 3 files changed, 21 insertions(+), 8 deletions(-)