hw/tpm/tpm_util.h | 17 ++++++++++- hw/tpm/tpm_emulator.c | 14 ++++----- hw/tpm/tpm_passthrough.c | 6 ++-- hw/tpm/tpm_util.c | 75 ++++++++++++++++++++++-------------------------- 4 files changed, 58 insertions(+), 54 deletions(-)
The new tpm-crb-test fails on sparc host:
TEST: tests/tpm-crb-test... (pid=230409)
/i386/tpm-crb/test:
Broken pipe
FAIL
GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
(pid=230423)
FAIL: tests/tpm-crb-test
and generates a new clang sanitizer runtime warning:
/home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
error: load of misaligned address 0x7fdc24c00002 for type 'const
uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
0x7fdc24c00002: note: pointer points here
<memory cannot be printed>
The sparc architecture does not allow misaligned loads and will
segfault if you try them. For example, this function:
static inline uint32_t tpm_cmd_get_size(const void *b)
{
return be32_to_cpu(*(const uint32_t *)(b + 2));
}
Should read,
return ldl_be_p(b + 2);
As a general rule you can't take an arbitrary pointer into a byte
buffer and try to interpret it as a structure or a pointer to a
larger-than-bytesize-data simply by casting the pointer.
Use this clean up as an opportunity to remove unnecessary temporary
buffers and casts.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/tpm/tpm_util.h | 17 ++++++++++-
hw/tpm/tpm_emulator.c | 14 ++++-----
hw/tpm/tpm_passthrough.c | 6 ++--
hw/tpm/tpm_util.c | 75 ++++++++++++++++++++++--------------------------
4 files changed, 58 insertions(+), 54 deletions(-)
diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
index 19b28474ae..c562140e52 100644
--- a/hw/tpm/tpm_util.h
+++ b/hw/tpm/tpm_util.h
@@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len);
int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
+static inline uint16_t tpm_cmd_get_tag(const void *b)
+{
+ return lduw_be_p(b);;
+}
+
static inline uint32_t tpm_cmd_get_size(const void *b)
{
- return be32_to_cpu(*(const uint32_t *)(b + 2));
+ return ldl_be_p(b + 2);;
+}
+
+static inline uint32_t tpm_cmd_get_ordinal(const void *b)
+{
+ return ldl_be_p(b + 6);;
+}
+
+static inline uint32_t tpm_cmd_get_errcode(const void *b)
+{
+ return ldl_be_p(b + 6);;
}
int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 35c78de5a9..a34a18ac7a 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
{
ssize_t ret;
bool is_selftest = false;
- const struct tpm_resp_hdr *hdr = NULL;
if (selftest_done) {
*selftest_done = false;
@@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
return -1;
}
- ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out, sizeof(*hdr),
- err);
+ ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
+ sizeof(struct tpm_resp_hdr), err);
if (ret != 0) {
return -1;
}
- hdr = (struct tpm_resp_hdr *)out;
- out += sizeof(*hdr);
- ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
- be32_to_cpu(hdr->len) - sizeof(*hdr) , err);
+ ret = qio_channel_read_all(tpm_emu->data_ioc,
+ (char *)out + sizeof(struct tpm_resp_hdr),
+ tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), err);
if (ret != 0) {
return -1;
}
if (is_selftest) {
- *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
+ *selftest_done = tpm_cmd_get_errcode(out) == 0;
}
return 0;
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 29142f38bb..537e11a3f9 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -87,7 +87,6 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
{
ssize_t ret;
bool is_selftest;
- const struct tpm_resp_hdr *hdr;
/* FIXME: protect shared variables or use other sync mechanism */
tpm_pt->tpm_op_canceled = false;
@@ -116,15 +115,14 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
strerror(errno), errno);
}
} else if (ret < sizeof(struct tpm_resp_hdr) ||
- be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
+ tpm_cmd_get_size(out) != ret) {
ret = -1;
error_report("tpm_passthrough: received invalid response "
"packet from TPM");
}
if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
- hdr = (struct tpm_resp_hdr *)out;
- *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
+ *selftest_done = tpm_cmd_get_errcode(out) == 0;
}
err_exit:
diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
index 747075e244..8abde59915 100644
--- a/hw/tpm/tpm_util.c
+++ b/hw/tpm/tpm_util.c
@@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
{
if (out_len >= sizeof(struct tpm_resp_hdr)) {
- struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
-
- resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
- resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
- resp->errcode = cpu_to_be32(TPM_FAIL);
+ stw_be_p(out, TPM_TAG_RSP_COMMAND);
+ stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
+ stl_be_p(out + 6, TPM_FAIL);
}
}
bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
{
- struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
-
- if (in_len >= sizeof(*hdr)) {
- return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
+ if (in_len >= sizeof(struct tpm_req_hdr)) {
+ return tpm_cmd_get_ordinal(in) == TPM_ORD_ContinueSelfTest;
}
return false;
@@ -129,12 +125,11 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
* Send request to a TPM device. We expect a response within one second.
*/
static int tpm_util_request(int fd,
- unsigned char *request,
+ const void *request,
size_t requestlen,
- unsigned char *response,
+ void *response,
size_t responselen)
{
- struct tpm_resp_hdr *resp;
fd_set readfds;
int n;
struct timeval tv = {
@@ -164,9 +159,8 @@ static int tpm_util_request(int fd,
return -EFAULT;
}
- resp = (struct tpm_resp_hdr *)response;
/* check the header */
- if (be32_to_cpu(resp->len) != n) {
+ if (tpm_cmd_get_size(response) != n) {
return -EMSGSIZE;
}
@@ -178,12 +172,11 @@ static int tpm_util_request(int fd,
* (error response is fine).
*/
static int tpm_util_test(int fd,
- unsigned char *request,
+ const void *request,
size_t requestlen,
uint16_t *return_tag)
{
- struct tpm_resp_hdr *resp;
- unsigned char buf[1024];
+ char buf[1024];
ssize_t ret;
ret = tpm_util_request(fd, request, requestlen,
@@ -192,8 +185,7 @@ static int tpm_util_test(int fd,
return ret;
}
- resp = (struct tpm_resp_hdr *)buf;
- *return_tag = be16_to_cpu(resp->tag);
+ *return_tag = tpm_cmd_get_tag(buf);
return 0;
}
@@ -228,7 +220,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
int ret;
/* Send TPM 2 command */
- ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2,
+ ret = tpm_util_test(tpm_fd, &test_req_tpm2,
sizeof(test_req_tpm2), &return_tag);
/* TPM 2 would respond with a tag of TPM2_ST_NO_SESSIONS */
if (!ret && return_tag == TPM2_ST_NO_SESSIONS) {
@@ -237,7 +229,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
}
/* Send TPM 1.2 command */
- ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req,
+ ret = tpm_util_test(tpm_fd, &test_req,
sizeof(test_req), &return_tag);
if (!ret && return_tag == TPM_TAG_RSP_COMMAND) {
*tpm_version = TPM_VERSION_1_2;
@@ -253,7 +245,6 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
size_t *buffersize)
{
- unsigned char buf[1024];
int ret;
switch (tpm_version) {
@@ -277,26 +268,27 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
struct tpm_resp_hdr hdr;
uint32_t len;
uint32_t buffersize;
- } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
+ } QEMU_PACKED tpm_resp;
- ret = tpm_util_request(tpm_fd, (unsigned char *)&tpm_get_buffer_size,
- sizeof(tpm_get_buffer_size), buf, sizeof(buf));
+ ret = tpm_util_request(tpm_fd, &tpm_get_buffer_size,
+ sizeof(tpm_get_buffer_size),
+ &tpm_resp, sizeof(tpm_resp));
if (ret < 0) {
return ret;
}
- if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
- be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
+ if (be32_to_cpu(tpm_resp.hdr.len) != sizeof(tpm_resp) ||
+ be32_to_cpu(tpm_resp.len) != sizeof(uint32_t)) {
DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
- be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
+ be32_to_cpu(tpm_resp.hdr.len), sizeof(tpm_resp));
DPRINTF("tpm_resp->len = %u, expected = %zu\n",
- be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
+ be32_to_cpu(tpm_resp.len), sizeof(uint32_t));
error_report("tpm_util: Got unexpected response to "
"TPM_GetCapability; errcode: 0x%x",
- be32_to_cpu(tpm_resp->hdr.errcode));
+ be32_to_cpu(tpm_resp.hdr.errcode));
return -EFAULT;
}
- *buffersize = be32_to_cpu(tpm_resp->buffersize);
+ *buffersize = be32_to_cpu(tpm_resp.buffersize);
break;
}
case TPM_VERSION_2_0: {
@@ -324,27 +316,28 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
uint32_t value1;
uint32_t property2;
uint32_t value2;
- } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
+ } QEMU_PACKED tpm2_resp;
- ret = tpm_util_request(tpm_fd, (unsigned char *)&tpm2_get_buffer_size,
- sizeof(tpm2_get_buffer_size), buf, sizeof(buf));
+ ret = tpm_util_request(tpm_fd, &tpm2_get_buffer_size,
+ sizeof(tpm2_get_buffer_size),
+ &tpm2_resp, sizeof(tpm2_resp));
if (ret < 0) {
return ret;
}
- if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
- be32_to_cpu(tpm2_resp->count) != 2) {
+ if (be32_to_cpu(tpm2_resp.hdr.len) != sizeof(tpm2_resp) ||
+ be32_to_cpu(tpm2_resp.count) != 2) {
DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
- be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
+ be32_to_cpu(tpm2_resp.hdr.len), sizeof(tpm2_resp));
DPRINTF("tpm2_resp->len = %u, expected = %u\n",
- be32_to_cpu(tpm2_resp->count), 2);
+ be32_to_cpu(tpm2_resp.count), 2);
error_report("tpm_util: Got unexpected response to "
"TPM2_GetCapability; errcode: 0x%x",
- be32_to_cpu(tpm2_resp->hdr.errcode));
+ be32_to_cpu(tpm2_resp.hdr.errcode));
return -EFAULT;
}
- *buffersize = MAX(be32_to_cpu(tpm2_resp->value1),
- be32_to_cpu(tpm2_resp->value2));
+ *buffersize = MAX(be32_to_cpu(tpm2_resp.value1),
+ be32_to_cpu(tpm2_resp.value2));
break;
}
case TPM_VERSION_UNSPEC:
--
2.16.0.rc1.1.gef27df75a1
On 01/29/2018 07:32 AM, Marc-André Lureau wrote:
> The new tpm-crb-test fails on sparc host:
>
> TEST: tests/tpm-crb-test... (pid=230409)
> /i386/tpm-crb/test:
> Broken pipe
> FAIL
> GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
> (pid=230423)
> FAIL: tests/tpm-crb-test
>
> and generates a new clang sanitizer runtime warning:
>
> /home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
> error: load of misaligned address 0x7fdc24c00002 for type 'const
> uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
> 0x7fdc24c00002: note: pointer points here
> <memory cannot be printed>
>
> The sparc architecture does not allow misaligned loads and will
> segfault if you try them. For example, this function:
>
> static inline uint32_t tpm_cmd_get_size(const void *b)
> {
> return be32_to_cpu(*(const uint32_t *)(b + 2));
> }
>
> Should read,
> return ldl_be_p(b + 2);
>
> As a general rule you can't take an arbitrary pointer into a byte
> buffer and try to interpret it as a structure or a pointer to a
> larger-than-bytesize-data simply by casting the pointer.
>
> Use this clean up as an opportunity to remove unnecessary temporary
> buffers and casts.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/tpm/tpm_util.h | 17 ++++++++++-
> hw/tpm/tpm_emulator.c | 14 ++++-----
> hw/tpm/tpm_passthrough.c | 6 ++--
> hw/tpm/tpm_util.c | 75 ++++++++++++++++++++++--------------------------
> 4 files changed, 58 insertions(+), 54 deletions(-)
>
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index 19b28474ae..c562140e52 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len);
>
> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>
> +static inline uint16_t tpm_cmd_get_tag(const void *b)
> +{
> + return lduw_be_p(b);;
> +}
> +
> static inline uint32_t tpm_cmd_get_size(const void *b)
> {
> - return be32_to_cpu(*(const uint32_t *)(b + 2));
> + return ldl_be_p(b + 2);;
Why are there these two ';;' ?
> +}
> +
> +static inline uint32_t tpm_cmd_get_ordinal(const void *b)
> +{
> + return ldl_be_p(b + 6);;
> +}
> +
> +static inline uint32_t tpm_cmd_get_errcode(const void *b)
> +{
> + return ldl_be_p(b + 6);;
> }
>
> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 35c78de5a9..a34a18ac7a 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
> {
> ssize_t ret;
> bool is_selftest = false;
> - const struct tpm_resp_hdr *hdr = NULL;
>
> if (selftest_done) {
> *selftest_done = false;
> @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
> return -1;
> }
>
> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out, sizeof(*hdr),
> - err);
> + ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
> + sizeof(struct tpm_resp_hdr), err);
> if (ret != 0) {
> return -1;
> }
>
> - hdr = (struct tpm_resp_hdr *)out;
> - out += sizeof(*hdr);
> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
> - be32_to_cpu(hdr->len) - sizeof(*hdr) , err);
> + ret = qio_channel_read_all(tpm_emu->data_ioc,
> + (char *)out + sizeof(struct tpm_resp_hdr),
> + tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), err);
> if (ret != 0) {
> return -1;
> }
>
> if (is_selftest) {
> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
> }
>
> return 0;
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 29142f38bb..537e11a3f9 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -87,7 +87,6 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> {
> ssize_t ret;
> bool is_selftest;
> - const struct tpm_resp_hdr *hdr;
>
> /* FIXME: protect shared variables or use other sync mechanism */
> tpm_pt->tpm_op_canceled = false;
> @@ -116,15 +115,14 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
> strerror(errno), errno);
> }
> } else if (ret < sizeof(struct tpm_resp_hdr) ||
> - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
> + tpm_cmd_get_size(out) != ret) {
> ret = -1;
> error_report("tpm_passthrough: received invalid response "
> "packet from TPM");
> }
>
> if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
> - hdr = (struct tpm_resp_hdr *)out;
> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
> }
>
> err_exit:
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 747075e244..8abde59915 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
> void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
> {
> if (out_len >= sizeof(struct tpm_resp_hdr)) {
> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
> -
> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
> - resp->errcode = cpu_to_be32(TPM_FAIL);
> + stw_be_p(out, TPM_TAG_RSP_COMMAND);
> + stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
> + stl_be_p(out + 6, TPM_FAIL);
Since we wrapped the getters we should wrap the setters as well.
tpm_cmd_set_len(), tpm_cmd_set_errcode.
> }
> }
>
> bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
> {
> - struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
> -
> - if (in_len >= sizeof(*hdr)) {
> - return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
> + if (in_len >= sizeof(struct tpm_req_hdr)) {
> + return tpm_cmd_get_ordinal(in) == TPM_ORD_ContinueSelfTest;
> }
>
> return false;
> @@ -129,12 +125,11 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
> * Send request to a TPM device. We expect a response within one second.
> */
> static int tpm_util_request(int fd,
> - unsigned char *request,
> + const void *request,
> size_t requestlen,
> - unsigned char *response,
> + void *response,
> size_t responselen)
> {
> - struct tpm_resp_hdr *resp;
> fd_set readfds;
> int n;
> struct timeval tv = {
> @@ -164,9 +159,8 @@ static int tpm_util_request(int fd,
> return -EFAULT;
> }
>
> - resp = (struct tpm_resp_hdr *)response;
> /* check the header */
> - if (be32_to_cpu(resp->len) != n) {
> + if (tpm_cmd_get_size(response) != n) {
> return -EMSGSIZE;
> }
>
> @@ -178,12 +172,11 @@ static int tpm_util_request(int fd,
> * (error response is fine).
> */
> static int tpm_util_test(int fd,
> - unsigned char *request,
> + const void *request,
> size_t requestlen,
> uint16_t *return_tag)
> {
> - struct tpm_resp_hdr *resp;
> - unsigned char buf[1024];
> + char buf[1024];
> ssize_t ret;
>
> ret = tpm_util_request(fd, request, requestlen,
> @@ -192,8 +185,7 @@ static int tpm_util_test(int fd,
> return ret;
> }
>
> - resp = (struct tpm_resp_hdr *)buf;
> - *return_tag = be16_to_cpu(resp->tag);
> + *return_tag = tpm_cmd_get_tag(buf);
>
> return 0;
> }
> @@ -228,7 +220,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
> int ret;
>
> /* Send TPM 2 command */
> - ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2,
> + ret = tpm_util_test(tpm_fd, &test_req_tpm2,
> sizeof(test_req_tpm2), &return_tag);
> /* TPM 2 would respond with a tag of TPM2_ST_NO_SESSIONS */
> if (!ret && return_tag == TPM2_ST_NO_SESSIONS) {
> @@ -237,7 +229,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
> }
>
> /* Send TPM 1.2 command */
> - ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req,
> + ret = tpm_util_test(tpm_fd, &test_req,
> sizeof(test_req), &return_tag);
> if (!ret && return_tag == TPM_TAG_RSP_COMMAND) {
> *tpm_version = TPM_VERSION_1_2;
> @@ -253,7 +245,6 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> size_t *buffersize)
> {
> - unsigned char buf[1024];
> int ret;
>
> switch (tpm_version) {
> @@ -277,26 +268,27 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> struct tpm_resp_hdr hdr;
> uint32_t len;
> uint32_t buffersize;
> - } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
> + } QEMU_PACKED tpm_resp;
>
> - ret = tpm_util_request(tpm_fd, (unsigned char *)&tpm_get_buffer_size,
> - sizeof(tpm_get_buffer_size), buf, sizeof(buf));
> + ret = tpm_util_request(tpm_fd, &tpm_get_buffer_size,
> + sizeof(tpm_get_buffer_size),
> + &tpm_resp, sizeof(tpm_resp));
> if (ret < 0) {
> return ret;
> }
>
> - if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
> - be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
> + if (be32_to_cpu(tpm_resp.hdr.len) != sizeof(tpm_resp) ||
> + be32_to_cpu(tpm_resp.len) != sizeof(uint32_t)) {
> DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
> - be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
> + be32_to_cpu(tpm_resp.hdr.len), sizeof(tpm_resp));
> DPRINTF("tpm_resp->len = %u, expected = %zu\n",
> - be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
> + be32_to_cpu(tpm_resp.len), sizeof(uint32_t));
> error_report("tpm_util: Got unexpected response to "
> "TPM_GetCapability; errcode: 0x%x",
> - be32_to_cpu(tpm_resp->hdr.errcode));
> + be32_to_cpu(tpm_resp.hdr.errcode));
> return -EFAULT;
> }
> - *buffersize = be32_to_cpu(tpm_resp->buffersize);
> + *buffersize = be32_to_cpu(tpm_resp.buffersize);
> break;
> }
> case TPM_VERSION_2_0: {
> @@ -324,27 +316,28 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> uint32_t value1;
> uint32_t property2;
> uint32_t value2;
> - } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
> + } QEMU_PACKED tpm2_resp;
>
> - ret = tpm_util_request(tpm_fd, (unsigned char *)&tpm2_get_buffer_size,
> - sizeof(tpm2_get_buffer_size), buf, sizeof(buf));
> + ret = tpm_util_request(tpm_fd, &tpm2_get_buffer_size,
> + sizeof(tpm2_get_buffer_size),
> + &tpm2_resp, sizeof(tpm2_resp));
> if (ret < 0) {
> return ret;
> }
>
> - if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
> - be32_to_cpu(tpm2_resp->count) != 2) {
> + if (be32_to_cpu(tpm2_resp.hdr.len) != sizeof(tpm2_resp) ||
> + be32_to_cpu(tpm2_resp.count) != 2) {
> DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
> - be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
> + be32_to_cpu(tpm2_resp.hdr.len), sizeof(tpm2_resp));
> DPRINTF("tpm2_resp->len = %u, expected = %u\n",
> - be32_to_cpu(tpm2_resp->count), 2);
> + be32_to_cpu(tpm2_resp.count), 2);
> error_report("tpm_util: Got unexpected response to "
> "TPM2_GetCapability; errcode: 0x%x",
> - be32_to_cpu(tpm2_resp->hdr.errcode));
> + be32_to_cpu(tpm2_resp.hdr.errcode));
> return -EFAULT;
> }
> - *buffersize = MAX(be32_to_cpu(tpm2_resp->value1),
> - be32_to_cpu(tpm2_resp->value2));
> + *buffersize = MAX(be32_to_cpu(tpm2_resp.value1),
> + be32_to_cpu(tpm2_resp.value2));
> break;
> }
> case TPM_VERSION_UNSPEC:
Hi
On Mon, Jan 29, 2018 at 6:41 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 01/29/2018 07:32 AM, Marc-André Lureau wrote:
>>
>> The new tpm-crb-test fails on sparc host:
>>
>> TEST: tests/tpm-crb-test... (pid=230409)
>> /i386/tpm-crb/test:
>> Broken pipe
>> FAIL
>> GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
>> (pid=230423)
>> FAIL: tests/tpm-crb-test
>>
>> and generates a new clang sanitizer runtime warning:
>>
>> /home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
>> error: load of misaligned address 0x7fdc24c00002 for type 'const
>> uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
>> 0x7fdc24c00002: note: pointer points here
>> <memory cannot be printed>
>>
>> The sparc architecture does not allow misaligned loads and will
>> segfault if you try them. For example, this function:
>>
>> static inline uint32_t tpm_cmd_get_size(const void *b)
>> {
>> return be32_to_cpu(*(const uint32_t *)(b + 2));
>> }
>>
>> Should read,
>> return ldl_be_p(b + 2);
>>
>> As a general rule you can't take an arbitrary pointer into a byte
>> buffer and try to interpret it as a structure or a pointer to a
>> larger-than-bytesize-data simply by casting the pointer.
>>
>> Use this clean up as an opportunity to remove unnecessary temporary
>> buffers and casts.
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> hw/tpm/tpm_util.h | 17 ++++++++++-
>> hw/tpm/tpm_emulator.c | 14 ++++-----
>> hw/tpm/tpm_passthrough.c | 6 ++--
>> hw/tpm/tpm_util.c | 75
>> ++++++++++++++++++++++--------------------------
>> 4 files changed, 58 insertions(+), 54 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
>> index 19b28474ae..c562140e52 100644
>> --- a/hw/tpm/tpm_util.h
>> +++ b/hw/tpm/tpm_util.h
>> @@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t
>> in_len);
>>
>> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>>
>> +static inline uint16_t tpm_cmd_get_tag(const void *b)
>> +{
>> + return lduw_be_p(b);;
>> +}
>> +
>> static inline uint32_t tpm_cmd_get_size(const void *b)
>> {
>> - return be32_to_cpu(*(const uint32_t *)(b + 2));
>> + return ldl_be_p(b + 2);;
>
>
> Why are there these two ';;' ?
>
obvious typo (repeated..)
>
>> +}
>> +
>> +static inline uint32_t tpm_cmd_get_ordinal(const void *b)
>> +{
>> + return ldl_be_p(b + 6);;
>> +}
>> +
>> +static inline uint32_t tpm_cmd_get_errcode(const void *b)
>> +{
>> + return ldl_be_p(b + 6);;
>> }
>>
>> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>> index 35c78de5a9..a34a18ac7a 100644
>> --- a/hw/tpm/tpm_emulator.c
>> +++ b/hw/tpm/tpm_emulator.c
>> @@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>> *tpm_emu,
>> {
>> ssize_t ret;
>> bool is_selftest = false;
>> - const struct tpm_resp_hdr *hdr = NULL;
>>
>> if (selftest_done) {
>> *selftest_done = false;
>> @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>> *tpm_emu,
>> return -1;
>> }
>>
>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>> sizeof(*hdr),
>> - err);
>> + ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>> + sizeof(struct tpm_resp_hdr), err);
>> if (ret != 0) {
>> return -1;
>> }
>>
>> - hdr = (struct tpm_resp_hdr *)out;
>> - out += sizeof(*hdr);
>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>> - be32_to_cpu(hdr->len) - sizeof(*hdr) ,
>> err);
>> + ret = qio_channel_read_all(tpm_emu->data_ioc,
>> + (char *)out + sizeof(struct tpm_resp_hdr),
>> + tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), err);
>> if (ret != 0) {
>> return -1;
>> }
>>
>> if (is_selftest) {
>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>> }
>>
>> return 0;
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index 29142f38bb..537e11a3f9 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -87,7 +87,6 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState
>> *tpm_pt,
>> {
>> ssize_t ret;
>> bool is_selftest;
>> - const struct tpm_resp_hdr *hdr;
>>
>> /* FIXME: protect shared variables or use other sync mechanism */
>> tpm_pt->tpm_op_canceled = false;
>> @@ -116,15 +115,14 @@ static int
>> tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>> strerror(errno), errno);
>> }
>> } else if (ret < sizeof(struct tpm_resp_hdr) ||
>> - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
>> + tpm_cmd_get_size(out) != ret) {
>> ret = -1;
>> error_report("tpm_passthrough: received invalid response "
>> "packet from TPM");
>> }
>>
>> if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
>> - hdr = (struct tpm_resp_hdr *)out;
>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>> }
>>
>> err_exit:
>> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
>> index 747075e244..8abde59915 100644
>> --- a/hw/tpm/tpm_util.c
>> +++ b/hw/tpm/tpm_util.c
>> @@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
>> void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
>> {
>> if (out_len >= sizeof(struct tpm_resp_hdr)) {
>> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
>> -
>> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
>> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
>> - resp->errcode = cpu_to_be32(TPM_FAIL);
>> + stw_be_p(out, TPM_TAG_RSP_COMMAND);
>> + stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
>> + stl_be_p(out + 6, TPM_FAIL);
>
>
> Since we wrapped the getters we should wrap the setters as well.
> tpm_cmd_set_len(), tpm_cmd_set_errcode.
They were not as widely used (there was only a getter so far), but
that makes sense too. Could be done on top.
>
>
>> }
>> }
>>
>> bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
>> {
>> - struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
>> -
>> - if (in_len >= sizeof(*hdr)) {
>> - return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
>> + if (in_len >= sizeof(struct tpm_req_hdr)) {
>> + return tpm_cmd_get_ordinal(in) == TPM_ORD_ContinueSelfTest;
>> }
>>
>> return false;
>> @@ -129,12 +125,11 @@ bool tpm_util_is_selftest(const uint8_t *in,
>> uint32_t in_len)
>> * Send request to a TPM device. We expect a response within one second.
>> */
>> static int tpm_util_request(int fd,
>> - unsigned char *request,
>> + const void *request,
>> size_t requestlen,
>> - unsigned char *response,
>> + void *response,
>> size_t responselen)
>> {
>> - struct tpm_resp_hdr *resp;
>> fd_set readfds;
>> int n;
>> struct timeval tv = {
>> @@ -164,9 +159,8 @@ static int tpm_util_request(int fd,
>> return -EFAULT;
>> }
>>
>> - resp = (struct tpm_resp_hdr *)response;
>> /* check the header */
>> - if (be32_to_cpu(resp->len) != n) {
>> + if (tpm_cmd_get_size(response) != n) {
>> return -EMSGSIZE;
>> }
>>
>> @@ -178,12 +172,11 @@ static int tpm_util_request(int fd,
>> * (error response is fine).
>> */
>> static int tpm_util_test(int fd,
>> - unsigned char *request,
>> + const void *request,
>> size_t requestlen,
>> uint16_t *return_tag)
>> {
>> - struct tpm_resp_hdr *resp;
>> - unsigned char buf[1024];
>> + char buf[1024];
>> ssize_t ret;
>>
>> ret = tpm_util_request(fd, request, requestlen,
>> @@ -192,8 +185,7 @@ static int tpm_util_test(int fd,
>> return ret;
>> }
>>
>> - resp = (struct tpm_resp_hdr *)buf;
>> - *return_tag = be16_to_cpu(resp->tag);
>> + *return_tag = tpm_cmd_get_tag(buf);
>>
>> return 0;
>> }
>> @@ -228,7 +220,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion
>> *tpm_version)
>> int ret;
>>
>> /* Send TPM 2 command */
>> - ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2,
>> + ret = tpm_util_test(tpm_fd, &test_req_tpm2,
>> sizeof(test_req_tpm2), &return_tag);
>> /* TPM 2 would respond with a tag of TPM2_ST_NO_SESSIONS */
>> if (!ret && return_tag == TPM2_ST_NO_SESSIONS) {
>> @@ -237,7 +229,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion
>> *tpm_version)
>> }
>>
>> /* Send TPM 1.2 command */
>> - ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req,
>> + ret = tpm_util_test(tpm_fd, &test_req,
>> sizeof(test_req), &return_tag);
>> if (!ret && return_tag == TPM_TAG_RSP_COMMAND) {
>> *tpm_version = TPM_VERSION_1_2;
>> @@ -253,7 +245,6 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion
>> *tpm_version)
>> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>> size_t *buffersize)
>> {
>> - unsigned char buf[1024];
>> int ret;
>>
>> switch (tpm_version) {
>> @@ -277,26 +268,27 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion
>> tpm_version,
>> struct tpm_resp_hdr hdr;
>> uint32_t len;
>> uint32_t buffersize;
>> - } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
>> + } QEMU_PACKED tpm_resp;
>>
>> - ret = tpm_util_request(tpm_fd, (unsigned char
>> *)&tpm_get_buffer_size,
>> - sizeof(tpm_get_buffer_size), buf,
>> sizeof(buf));
>> + ret = tpm_util_request(tpm_fd, &tpm_get_buffer_size,
>> + sizeof(tpm_get_buffer_size),
>> + &tpm_resp, sizeof(tpm_resp));
>> if (ret < 0) {
>> return ret;
>> }
>>
>> - if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
>> - be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
>> + if (be32_to_cpu(tpm_resp.hdr.len) != sizeof(tpm_resp) ||
>> + be32_to_cpu(tpm_resp.len) != sizeof(uint32_t)) {
>> DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
>> - be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
>> + be32_to_cpu(tpm_resp.hdr.len), sizeof(tpm_resp));
>> DPRINTF("tpm_resp->len = %u, expected = %zu\n",
>> - be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
>> + be32_to_cpu(tpm_resp.len), sizeof(uint32_t));
>> error_report("tpm_util: Got unexpected response to "
>> "TPM_GetCapability; errcode: 0x%x",
>> - be32_to_cpu(tpm_resp->hdr.errcode));
>> + be32_to_cpu(tpm_resp.hdr.errcode));
>> return -EFAULT;
>> }
>> - *buffersize = be32_to_cpu(tpm_resp->buffersize);
>> + *buffersize = be32_to_cpu(tpm_resp.buffersize);
>> break;
>> }
>> case TPM_VERSION_2_0: {
>> @@ -324,27 +316,28 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion
>> tpm_version,
>> uint32_t value1;
>> uint32_t property2;
>> uint32_t value2;
>> - } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size
>> *)buf;
>> + } QEMU_PACKED tpm2_resp;
>>
>> - ret = tpm_util_request(tpm_fd, (unsigned char
>> *)&tpm2_get_buffer_size,
>> - sizeof(tpm2_get_buffer_size), buf,
>> sizeof(buf));
>> + ret = tpm_util_request(tpm_fd, &tpm2_get_buffer_size,
>> + sizeof(tpm2_get_buffer_size),
>> + &tpm2_resp, sizeof(tpm2_resp));
>> if (ret < 0) {
>> return ret;
>> }
>>
>> - if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
>> - be32_to_cpu(tpm2_resp->count) != 2) {
>> + if (be32_to_cpu(tpm2_resp.hdr.len) != sizeof(tpm2_resp) ||
>> + be32_to_cpu(tpm2_resp.count) != 2) {
>> DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
>> - be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
>> + be32_to_cpu(tpm2_resp.hdr.len), sizeof(tpm2_resp));
>> DPRINTF("tpm2_resp->len = %u, expected = %u\n",
>> - be32_to_cpu(tpm2_resp->count), 2);
>> + be32_to_cpu(tpm2_resp.count), 2);
>> error_report("tpm_util: Got unexpected response to "
>> "TPM2_GetCapability; errcode: 0x%x",
>> - be32_to_cpu(tpm2_resp->hdr.errcode));
>> + be32_to_cpu(tpm2_resp.hdr.errcode));
>> return -EFAULT;
>> }
>> - *buffersize = MAX(be32_to_cpu(tpm2_resp->value1),
>> - be32_to_cpu(tpm2_resp->value2));
>> + *buffersize = MAX(be32_to_cpu(tpm2_resp.value1),
>> + be32_to_cpu(tpm2_resp.value2));
>> break;
>> }
>> case TPM_VERSION_UNSPEC:
>
>
>
On 01/29/2018 12:51 PM, Marc-Andre Lureau wrote:
> Hi
>
> On Mon, Jan 29, 2018 at 6:41 PM, Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> On 01/29/2018 07:32 AM, Marc-André Lureau wrote:
>>> The new tpm-crb-test fails on sparc host:
>>>
>>> TEST: tests/tpm-crb-test... (pid=230409)
>>> /i386/tpm-crb/test:
>>> Broken pipe
>>> FAIL
>>> GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
>>> (pid=230423)
>>> FAIL: tests/tpm-crb-test
>>>
>>> and generates a new clang sanitizer runtime warning:
>>>
>>> /home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
>>> error: load of misaligned address 0x7fdc24c00002 for type 'const
>>> uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
>>> 0x7fdc24c00002: note: pointer points here
>>> <memory cannot be printed>
>>>
>>> The sparc architecture does not allow misaligned loads and will
>>> segfault if you try them. For example, this function:
>>>
>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>> {
>>> return be32_to_cpu(*(const uint32_t *)(b + 2));
>>> }
>>>
>>> Should read,
>>> return ldl_be_p(b + 2);
>>>
>>> As a general rule you can't take an arbitrary pointer into a byte
>>> buffer and try to interpret it as a structure or a pointer to a
>>> larger-than-bytesize-data simply by casting the pointer.
>>>
>>> Use this clean up as an opportunity to remove unnecessary temporary
>>> buffers and casts.
>>>
>>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>> hw/tpm/tpm_util.h | 17 ++++++++++-
>>> hw/tpm/tpm_emulator.c | 14 ++++-----
>>> hw/tpm/tpm_passthrough.c | 6 ++--
>>> hw/tpm/tpm_util.c | 75
>>> ++++++++++++++++++++++--------------------------
>>> 4 files changed, 58 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
>>> index 19b28474ae..c562140e52 100644
>>> --- a/hw/tpm/tpm_util.h
>>> +++ b/hw/tpm/tpm_util.h
>>> @@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t
>>> in_len);
>>>
>>> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>>>
>>> +static inline uint16_t tpm_cmd_get_tag(const void *b)
>>> +{
>>> + return lduw_be_p(b);;
>>> +}
>>> +
>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>> {
>>> - return be32_to_cpu(*(const uint32_t *)(b + 2));
>>> + return ldl_be_p(b + 2);;
>>
>> Why are there these two ';;' ?
>>
> obvious typo (repeated..)
>
>>> +}
>>> +
>>> +static inline uint32_t tpm_cmd_get_ordinal(const void *b)
>>> +{
>>> + return ldl_be_p(b + 6);;
>>> +}
>>> +
>>> +static inline uint32_t tpm_cmd_get_errcode(const void *b)
>>> +{
>>> + return ldl_be_p(b + 6);;
>>> }
>>>
>>> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>>> index 35c78de5a9..a34a18ac7a 100644
>>> --- a/hw/tpm/tpm_emulator.c
>>> +++ b/hw/tpm/tpm_emulator.c
>>> @@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>> *tpm_emu,
>>> {
>>> ssize_t ret;
>>> bool is_selftest = false;
>>> - const struct tpm_resp_hdr *hdr = NULL;
>>>
>>> if (selftest_done) {
>>> *selftest_done = false;
>>> @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>> *tpm_emu,
>>> return -1;
>>> }
>>>
>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>> sizeof(*hdr),
>>> - err);
>>> + ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>> + sizeof(struct tpm_resp_hdr), err);
>>> if (ret != 0) {
>>> return -1;
>>> }
>>>
>>> - hdr = (struct tpm_resp_hdr *)out;
>>> - out += sizeof(*hdr);
>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>> - be32_to_cpu(hdr->len) - sizeof(*hdr) ,
>>> err);
>>> + ret = qio_channel_read_all(tpm_emu->data_ioc,
>>> + (char *)out + sizeof(struct tpm_resp_hdr),
>>> + tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), err);
>>> if (ret != 0) {
>>> return -1;
>>> }
>>>
>>> if (is_selftest) {
>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>> }
>>>
>>> return 0;
>>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>>> index 29142f38bb..537e11a3f9 100644
>>> --- a/hw/tpm/tpm_passthrough.c
>>> +++ b/hw/tpm/tpm_passthrough.c
>>> @@ -87,7 +87,6 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState
>>> *tpm_pt,
>>> {
>>> ssize_t ret;
>>> bool is_selftest;
>>> - const struct tpm_resp_hdr *hdr;
>>>
>>> /* FIXME: protect shared variables or use other sync mechanism */
>>> tpm_pt->tpm_op_canceled = false;
>>> @@ -116,15 +115,14 @@ static int
>>> tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>>> strerror(errno), errno);
>>> }
>>> } else if (ret < sizeof(struct tpm_resp_hdr) ||
>>> - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
>>> + tpm_cmd_get_size(out) != ret) {
>>> ret = -1;
>>> error_report("tpm_passthrough: received invalid response "
>>> "packet from TPM");
>>> }
>>>
>>> if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
>>> - hdr = (struct tpm_resp_hdr *)out;
>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>> }
>>>
>>> err_exit:
>>> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
>>> index 747075e244..8abde59915 100644
>>> --- a/hw/tpm/tpm_util.c
>>> +++ b/hw/tpm/tpm_util.c
>>> @@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
>>> void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len)
>>> {
>>> if (out_len >= sizeof(struct tpm_resp_hdr)) {
>>> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
>>> -
>>> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
>>> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
>>> - resp->errcode = cpu_to_be32(TPM_FAIL);
>>> + stw_be_p(out, TPM_TAG_RSP_COMMAND);
>>> + stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
>>> + stl_be_p(out + 6, TPM_FAIL);
>>
>> Since we wrapped the getters we should wrap the setters as well.
>> tpm_cmd_set_len(), tpm_cmd_set_errcode.
> They were not as widely used (there was only a getter so far), but
> that makes sense too. Could be done on top.
Also please rebase on top of your series of patches. I had some problems
with tpm_passthrough.c around line 115. Error reporting there has changed.
Hi
On Mon, Jan 29, 2018 at 6:57 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 01/29/2018 12:51 PM, Marc-Andre Lureau wrote:
>>
>> Hi
>>
>> On Mon, Jan 29, 2018 at 6:41 PM, Stefan Berger
>> <stefanb@linux.vnet.ibm.com> wrote:
>>>
>>> On 01/29/2018 07:32 AM, Marc-André Lureau wrote:
>>>>
>>>> The new tpm-crb-test fails on sparc host:
>>>>
>>>> TEST: tests/tpm-crb-test... (pid=230409)
>>>> /i386/tpm-crb/test:
>>>> Broken pipe
>>>> FAIL
>>>> GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
>>>> (pid=230423)
>>>> FAIL: tests/tpm-crb-test
>>>>
>>>> and generates a new clang sanitizer runtime warning:
>>>>
>>>> /home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
>>>> error: load of misaligned address 0x7fdc24c00002 for type 'const
>>>> uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
>>>> 0x7fdc24c00002: note: pointer points here
>>>> <memory cannot be printed>
>>>>
>>>> The sparc architecture does not allow misaligned loads and will
>>>> segfault if you try them. For example, this function:
>>>>
>>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>>> {
>>>> return be32_to_cpu(*(const uint32_t *)(b + 2));
>>>> }
>>>>
>>>> Should read,
>>>> return ldl_be_p(b + 2);
>>>>
>>>> As a general rule you can't take an arbitrary pointer into a byte
>>>> buffer and try to interpret it as a structure or a pointer to a
>>>> larger-than-bytesize-data simply by casting the pointer.
>>>>
>>>> Use this clean up as an opportunity to remove unnecessary temporary
>>>> buffers and casts.
>>>>
>>>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> ---
>>>> hw/tpm/tpm_util.h | 17 ++++++++++-
>>>> hw/tpm/tpm_emulator.c | 14 ++++-----
>>>> hw/tpm/tpm_passthrough.c | 6 ++--
>>>> hw/tpm/tpm_util.c | 75
>>>> ++++++++++++++++++++++--------------------------
>>>> 4 files changed, 58 insertions(+), 54 deletions(-)
>>>>
>>>> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
>>>> index 19b28474ae..c562140e52 100644
>>>> --- a/hw/tpm/tpm_util.h
>>>> +++ b/hw/tpm/tpm_util.h
>>>> @@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t
>>>> in_len);
>>>>
>>>> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>>>>
>>>> +static inline uint16_t tpm_cmd_get_tag(const void *b)
>>>> +{
>>>> + return lduw_be_p(b);;
>>>> +}
>>>> +
>>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>>> {
>>>> - return be32_to_cpu(*(const uint32_t *)(b + 2));
>>>> + return ldl_be_p(b + 2);;
>>>
>>>
>>> Why are there these two ';;' ?
>>>
>> obvious typo (repeated..)
>>
>>>> +}
>>>> +
>>>> +static inline uint32_t tpm_cmd_get_ordinal(const void *b)
>>>> +{
>>>> + return ldl_be_p(b + 6);;
>>>> +}
>>>> +
>>>> +static inline uint32_t tpm_cmd_get_errcode(const void *b)
>>>> +{
>>>> + return ldl_be_p(b + 6);;
>>>> }
>>>>
>>>> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>>>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>>>> index 35c78de5a9..a34a18ac7a 100644
>>>> --- a/hw/tpm/tpm_emulator.c
>>>> +++ b/hw/tpm/tpm_emulator.c
>>>> @@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>>> *tpm_emu,
>>>> {
>>>> ssize_t ret;
>>>> bool is_selftest = false;
>>>> - const struct tpm_resp_hdr *hdr = NULL;
>>>>
>>>> if (selftest_done) {
>>>> *selftest_done = false;
>>>> @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>>> *tpm_emu,
>>>> return -1;
>>>> }
>>>>
>>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>> sizeof(*hdr),
>>>> - err);
>>>> + ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>> + sizeof(struct tpm_resp_hdr), err);
>>>> if (ret != 0) {
>>>> return -1;
>>>> }
>>>>
>>>> - hdr = (struct tpm_resp_hdr *)out;
>>>> - out += sizeof(*hdr);
>>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>> - be32_to_cpu(hdr->len) - sizeof(*hdr) ,
>>>> err);
>>>> + ret = qio_channel_read_all(tpm_emu->data_ioc,
>>>> + (char *)out + sizeof(struct tpm_resp_hdr),
>>>> + tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr),
>>>> err);
>>>> if (ret != 0) {
>>>> return -1;
>>>> }
>>>>
>>>> if (is_selftest) {
>>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>>> }
>>>>
>>>> return 0;
>>>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>>>> index 29142f38bb..537e11a3f9 100644
>>>> --- a/hw/tpm/tpm_passthrough.c
>>>> +++ b/hw/tpm/tpm_passthrough.c
>>>> @@ -87,7 +87,6 @@ static int
>>>> tpm_passthrough_unix_tx_bufs(TPMPassthruState
>>>> *tpm_pt,
>>>> {
>>>> ssize_t ret;
>>>> bool is_selftest;
>>>> - const struct tpm_resp_hdr *hdr;
>>>>
>>>> /* FIXME: protect shared variables or use other sync mechanism */
>>>> tpm_pt->tpm_op_canceled = false;
>>>> @@ -116,15 +115,14 @@ static int
>>>> tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>>>> strerror(errno), errno);
>>>> }
>>>> } else if (ret < sizeof(struct tpm_resp_hdr) ||
>>>> - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
>>>> + tpm_cmd_get_size(out) != ret) {
>>>> ret = -1;
>>>> error_report("tpm_passthrough: received invalid response "
>>>> "packet from TPM");
>>>> }
>>>>
>>>> if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
>>>> - hdr = (struct tpm_resp_hdr *)out;
>>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>>> }
>>>>
>>>> err_exit:
>>>> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
>>>> index 747075e244..8abde59915 100644
>>>> --- a/hw/tpm/tpm_util.c
>>>> +++ b/hw/tpm/tpm_util.c
>>>> @@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
>>>> void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t
>>>> out_len)
>>>> {
>>>> if (out_len >= sizeof(struct tpm_resp_hdr)) {
>>>> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
>>>> -
>>>> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
>>>> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
>>>> - resp->errcode = cpu_to_be32(TPM_FAIL);
>>>> + stw_be_p(out, TPM_TAG_RSP_COMMAND);
>>>> + stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
>>>> + stl_be_p(out + 6, TPM_FAIL);
>>>
>>>
>>> Since we wrapped the getters we should wrap the setters as well.
>>> tpm_cmd_set_len(), tpm_cmd_set_errcode.
>>
>> They were not as widely used (there was only a getter so far), but
>> that makes sense too. Could be done on top.
>
>
> Also please rebase on top of your series of patches. I had some problems
> with tpm_passthrough.c around line 115. Error reporting there has changed.
>
>
The patch is meant to be applied first on top of master, to avoid
intermediary regressions (it passes patchew:
http://patchew.org/QEMU/20180129123227.15778-1-marcandre.lureau@redhat.com/)
There was no conflicts after a rebase of my series. If it helps, I can
resend my pending tpm queue.
On 01/29/2018 01:02 PM, Marc-Andre Lureau wrote:
> Hi
>
> On Mon, Jan 29, 2018 at 6:57 PM, Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> On 01/29/2018 12:51 PM, Marc-Andre Lureau wrote:
>>> Hi
>>>
>>> On Mon, Jan 29, 2018 at 6:41 PM, Stefan Berger
>>> <stefanb@linux.vnet.ibm.com> wrote:
>>>> On 01/29/2018 07:32 AM, Marc-André Lureau wrote:
>>>>> The new tpm-crb-test fails on sparc host:
>>>>>
>>>>> TEST: tests/tpm-crb-test... (pid=230409)
>>>>> /i386/tpm-crb/test:
>>>>> Broken pipe
>>>>> FAIL
>>>>> GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85
>>>>> (pid=230423)
>>>>> FAIL: tests/tpm-crb-test
>>>>>
>>>>> and generates a new clang sanitizer runtime warning:
>>>>>
>>>>> /home/petmay01/linaro/qemu-for-merges/hw/tpm/tpm_util.h:36:24: runtime
>>>>> error: load of misaligned address 0x7fdc24c00002 for type 'const
>>>>> uint32_t' (aka 'const unsigned int'), which requires 4 byte alignment
>>>>> 0x7fdc24c00002: note: pointer points here
>>>>> <memory cannot be printed>
>>>>>
>>>>> The sparc architecture does not allow misaligned loads and will
>>>>> segfault if you try them. For example, this function:
>>>>>
>>>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>>>> {
>>>>> return be32_to_cpu(*(const uint32_t *)(b + 2));
>>>>> }
>>>>>
>>>>> Should read,
>>>>> return ldl_be_p(b + 2);
>>>>>
>>>>> As a general rule you can't take an arbitrary pointer into a byte
>>>>> buffer and try to interpret it as a structure or a pointer to a
>>>>> larger-than-bytesize-data simply by casting the pointer.
>>>>>
>>>>> Use this clean up as an opportunity to remove unnecessary temporary
>>>>> buffers and casts.
>>>>>
>>>>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>> ---
>>>>> hw/tpm/tpm_util.h | 17 ++++++++++-
>>>>> hw/tpm/tpm_emulator.c | 14 ++++-----
>>>>> hw/tpm/tpm_passthrough.c | 6 ++--
>>>>> hw/tpm/tpm_util.c | 75
>>>>> ++++++++++++++++++++++--------------------------
>>>>> 4 files changed, 58 insertions(+), 54 deletions(-)
>>>>>
>>>>> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
>>>>> index 19b28474ae..c562140e52 100644
>>>>> --- a/hw/tpm/tpm_util.h
>>>>> +++ b/hw/tpm/tpm_util.h
>>>>> @@ -31,9 +31,24 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t
>>>>> in_len);
>>>>>
>>>>> int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version);
>>>>>
>>>>> +static inline uint16_t tpm_cmd_get_tag(const void *b)
>>>>> +{
>>>>> + return lduw_be_p(b);;
>>>>> +}
>>>>> +
>>>>> static inline uint32_t tpm_cmd_get_size(const void *b)
>>>>> {
>>>>> - return be32_to_cpu(*(const uint32_t *)(b + 2));
>>>>> + return ldl_be_p(b + 2);;
>>>>
>>>> Why are there these two ';;' ?
>>>>
>>> obvious typo (repeated..)
>>>
>>>>> +}
>>>>> +
>>>>> +static inline uint32_t tpm_cmd_get_ordinal(const void *b)
>>>>> +{
>>>>> + return ldl_be_p(b + 6);;
>>>>> +}
>>>>> +
>>>>> +static inline uint32_t tpm_cmd_get_errcode(const void *b)
>>>>> +{
>>>>> + return ldl_be_p(b + 6);;
>>>>> }
>>>>>
>>>>> int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
>>>>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>>>>> index 35c78de5a9..a34a18ac7a 100644
>>>>> --- a/hw/tpm/tpm_emulator.c
>>>>> +++ b/hw/tpm/tpm_emulator.c
>>>>> @@ -120,7 +120,6 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>>>> *tpm_emu,
>>>>> {
>>>>> ssize_t ret;
>>>>> bool is_selftest = false;
>>>>> - const struct tpm_resp_hdr *hdr = NULL;
>>>>>
>>>>> if (selftest_done) {
>>>>> *selftest_done = false;
>>>>> @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator
>>>>> *tpm_emu,
>>>>> return -1;
>>>>> }
>>>>>
>>>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>>> sizeof(*hdr),
>>>>> - err);
>>>>> + ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>>> + sizeof(struct tpm_resp_hdr), err);
>>>>> if (ret != 0) {
>>>>> return -1;
>>>>> }
>>>>>
>>>>> - hdr = (struct tpm_resp_hdr *)out;
>>>>> - out += sizeof(*hdr);
>>>>> - ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
>>>>> - be32_to_cpu(hdr->len) - sizeof(*hdr) ,
>>>>> err);
>>>>> + ret = qio_channel_read_all(tpm_emu->data_ioc,
>>>>> + (char *)out + sizeof(struct tpm_resp_hdr),
>>>>> + tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr),
>>>>> err);
>>>>> if (ret != 0) {
>>>>> return -1;
>>>>> }
>>>>>
>>>>> if (is_selftest) {
>>>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>>>> }
>>>>>
>>>>> return 0;
>>>>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>>>>> index 29142f38bb..537e11a3f9 100644
>>>>> --- a/hw/tpm/tpm_passthrough.c
>>>>> +++ b/hw/tpm/tpm_passthrough.c
>>>>> @@ -87,7 +87,6 @@ static int
>>>>> tpm_passthrough_unix_tx_bufs(TPMPassthruState
>>>>> *tpm_pt,
>>>>> {
>>>>> ssize_t ret;
>>>>> bool is_selftest;
>>>>> - const struct tpm_resp_hdr *hdr;
>>>>>
>>>>> /* FIXME: protect shared variables or use other sync mechanism */
>>>>> tpm_pt->tpm_op_canceled = false;
>>>>> @@ -116,15 +115,14 @@ static int
>>>>> tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>>>>> strerror(errno), errno);
>>>>> }
>>>>> } else if (ret < sizeof(struct tpm_resp_hdr) ||
>>>>> - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) {
>>>>> + tpm_cmd_get_size(out) != ret) {
>>>>> ret = -1;
>>>>> error_report("tpm_passthrough: received invalid response "
>>>>> "packet from TPM");
>>>>> }
>>>>>
>>>>> if (is_selftest && (ret >= sizeof(struct tpm_resp_hdr))) {
>>>>> - hdr = (struct tpm_resp_hdr *)out;
>>>>> - *selftest_done = (be32_to_cpu(hdr->errcode) == 0);
>>>>> + *selftest_done = tpm_cmd_get_errcode(out) == 0;
>>>>> }
>>>>>
>>>>> err_exit:
>>>>> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
>>>>> index 747075e244..8abde59915 100644
>>>>> --- a/hw/tpm/tpm_util.c
>>>>> +++ b/hw/tpm/tpm_util.c
>>>>> @@ -106,20 +106,16 @@ const PropertyInfo qdev_prop_tpm = {
>>>>> void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t
>>>>> out_len)
>>>>> {
>>>>> if (out_len >= sizeof(struct tpm_resp_hdr)) {
>>>>> - struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
>>>>> -
>>>>> - resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
>>>>> - resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
>>>>> - resp->errcode = cpu_to_be32(TPM_FAIL);
>>>>> + stw_be_p(out, TPM_TAG_RSP_COMMAND);
>>>>> + stl_be_p(out + 2, sizeof(struct tpm_resp_hdr));
>>>>> + stl_be_p(out + 6, TPM_FAIL);
>>>>
>>>> Since we wrapped the getters we should wrap the setters as well.
>>>> tpm_cmd_set_len(), tpm_cmd_set_errcode.
>>> They were not as widely used (there was only a getter so far), but
>>> that makes sense too. Could be done on top.
>>
>> Also please rebase on top of your series of patches. I had some problems
>> with tpm_passthrough.c around line 115. Error reporting there has changed.
>>
>>
> The patch is meant to be applied first on top of master, to avoid
> intermediary regressions (it passes patchew:
> http://patchew.org/QEMU/20180129123227.15778-1-marcandre.lureau@redhat.com/)
>
> There was no conflicts after a rebase of my series. If it helps, I can
> resend my pending tpm queue.
>
Please resent the series.
Stefan
© 2016 - 2024 Red Hat, Inc.