From nobody Mon Apr 29 02:12:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1517229228723219.35202114798733; Mon, 29 Jan 2018 04:33:48 -0800 (PST) Received: from localhost ([::1]:34641 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eg8d1-0007Vz-Bp for importer@patchew.org; Mon, 29 Jan 2018 07:33:39 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47281) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eg8c6-00078n-0D for qemu-devel@nongnu.org; Mon, 29 Jan 2018 07:32:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eg8c2-0004WA-PS for qemu-devel@nongnu.org; Mon, 29 Jan 2018 07:32:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49982) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eg8c2-0004UQ-HE for qemu-devel@nongnu.org; Mon, 29 Jan 2018 07:32:38 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3CC4A10F3CC; Mon, 29 Jan 2018 12:32:37 +0000 (UTC) Received: from localhost (ovpn-112-47.ams2.redhat.com [10.36.112.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1C04B6FDC4; Mon, 29 Jan 2018 12:32:34 +0000 (UTC) From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= To: qemu-devel@nongnu.org, stefanb@linux.vnet.ibm.com Date: Mon, 29 Jan 2018 13:32:27 +0100 Message-Id: <20180129123227.15778-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 29 Jan 2018 12:32:37 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH] tpm: fix alignment issues X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The new tpm-crb-test fails on sparc host: TEST: tests/tpm-crb-test... (pid=3D230409) /i386/tpm-crb/test: Broken pipe FAIL GTester: last random seed: R02S29cea50247fe1efa59ee885a26d51a85 (pid=3D230423) 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 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 Signed-off-by: Marc-Andr=C3=A9 Lureau --- 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); =20 int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version); =20 +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);; } =20 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_e= mu, { ssize_t ret; bool is_selftest =3D false; - const struct tpm_resp_hdr *hdr =3D NULL; =20 if (selftest_done) { *selftest_done =3D false; @@ -132,22 +131,21 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm= _emu, return -1; } =20 - ret =3D qio_channel_read_all(tpm_emu->data_ioc, (char *)out, sizeof(*h= dr), - err); + ret =3D qio_channel_read_all(tpm_emu->data_ioc, (char *)out, + sizeof(struct tpm_resp_hdr), err); if (ret !=3D 0) { return -1; } =20 - hdr =3D (struct tpm_resp_hdr *)out; - out +=3D sizeof(*hdr); - ret =3D qio_channel_read_all(tpm_emu->data_ioc, (char *)out, - be32_to_cpu(hdr->len) - sizeof(*hdr) , err); + ret =3D 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 !=3D 0) { return -1; } =20 if (is_selftest) { - *selftest_done =3D (be32_to_cpu(hdr->errcode) =3D=3D 0); + *selftest_done =3D tpm_cmd_get_errcode(out) =3D=3D 0; } =20 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; =20 /* FIXME: protect shared variables or use other sync mechanism */ tpm_pt->tpm_op_canceled =3D false; @@ -116,15 +115,14 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruSt= ate *tpm_pt, strerror(errno), errno); } } else if (ret < sizeof(struct tpm_resp_hdr) || - be32_to_cpu(((struct tpm_resp_hdr *)out)->len) !=3D ret) { + tpm_cmd_get_size(out) !=3D ret) { ret =3D -1; error_report("tpm_passthrough: received invalid response " "packet from TPM"); } =20 if (is_selftest && (ret >=3D sizeof(struct tpm_resp_hdr))) { - hdr =3D (struct tpm_resp_hdr *)out; - *selftest_done =3D (be32_to_cpu(hdr->errcode) =3D=3D 0); + *selftest_done =3D tpm_cmd_get_errcode(out) =3D=3D 0; } =20 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 =3D { void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len) { if (out_len >=3D sizeof(struct tpm_resp_hdr)) { - struct tpm_resp_hdr *resp =3D (struct tpm_resp_hdr *)out; - - resp->tag =3D cpu_to_be16(TPM_TAG_RSP_COMMAND); - resp->len =3D cpu_to_be32(sizeof(struct tpm_resp_hdr)); - resp->errcode =3D 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); } } =20 bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len) { - struct tpm_req_hdr *hdr =3D (struct tpm_req_hdr *)in; - - if (in_len >=3D sizeof(*hdr)) { - return (be32_to_cpu(hdr->ordinal) =3D=3D TPM_ORD_ContinueSelfTest); + if (in_len >=3D sizeof(struct tpm_req_hdr)) { + return tpm_cmd_get_ordinal(in) =3D=3D TPM_ORD_ContinueSelfTest; } =20 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 =3D { @@ -164,9 +159,8 @@ static int tpm_util_request(int fd, return -EFAULT; } =20 - resp =3D (struct tpm_resp_hdr *)response; /* check the header */ - if (be32_to_cpu(resp->len) !=3D n) { + if (tpm_cmd_get_size(response) !=3D n) { return -EMSGSIZE; } =20 @@ -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; =20 ret =3D tpm_util_request(fd, request, requestlen, @@ -192,8 +185,7 @@ static int tpm_util_test(int fd, return ret; } =20 - resp =3D (struct tpm_resp_hdr *)buf; - *return_tag =3D be16_to_cpu(resp->tag); + *return_tag =3D tpm_cmd_get_tag(buf); =20 return 0; } @@ -228,7 +220,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_ve= rsion) int ret; =20 /* Send TPM 2 command */ - ret =3D tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2, + ret =3D 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 =3D=3D TPM2_ST_NO_SESSIONS) { @@ -237,7 +229,7 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_ve= rsion) } =20 /* Send TPM 1.2 command */ - ret =3D tpm_util_test(tpm_fd, (unsigned char *)&test_req, + ret =3D tpm_util_test(tpm_fd, &test_req, sizeof(test_req), &return_tag); if (!ret && return_tag =3D=3D TPM_TAG_RSP_COMMAND) { *tpm_version =3D TPM_VERSION_1_2; @@ -253,7 +245,6 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_ve= rsion) int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version, size_t *buffersize) { - unsigned char buf[1024]; int ret; =20 switch (tpm_version) { @@ -277,26 +268,27 @@ int tpm_util_get_buffer_size(int tpm_fd, TPMVersion t= pm_version, struct tpm_resp_hdr hdr; uint32_t len; uint32_t buffersize; - } QEMU_PACKED *tpm_resp =3D (struct tpm_resp_get_buffer_size *)buf; + } QEMU_PACKED tpm_resp; =20 - ret =3D tpm_util_request(tpm_fd, (unsigned char *)&tpm_get_buffer_= size, - sizeof(tpm_get_buffer_size), buf, sizeof(bu= f)); + ret =3D tpm_util_request(tpm_fd, &tpm_get_buffer_size, + sizeof(tpm_get_buffer_size), + &tpm_resp, sizeof(tpm_resp)); if (ret < 0) { return ret; } =20 - if (be32_to_cpu(tpm_resp->hdr.len) !=3D sizeof(*tpm_resp) || - be32_to_cpu(tpm_resp->len) !=3D sizeof(uint32_t)) { + if (be32_to_cpu(tpm_resp.hdr.len) !=3D sizeof(tpm_resp) || + be32_to_cpu(tpm_resp.len) !=3D sizeof(uint32_t)) { DPRINTF("tpm_resp->hdr.len =3D %u, expected =3D %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 =3D %u, expected =3D %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 =3D be32_to_cpu(tpm_resp->buffersize); + *buffersize =3D 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 t= pm_version, uint32_t value1; uint32_t property2; uint32_t value2; - } QEMU_PACKED *tpm2_resp =3D (struct tpm2_resp_get_buffer_size *)b= uf; + } QEMU_PACKED tpm2_resp; =20 - ret =3D tpm_util_request(tpm_fd, (unsigned char *)&tpm2_get_buffer= _size, - sizeof(tpm2_get_buffer_size), buf, sizeof(b= uf)); + ret =3D tpm_util_request(tpm_fd, &tpm2_get_buffer_size, + sizeof(tpm2_get_buffer_size), + &tpm2_resp, sizeof(tpm2_resp)); if (ret < 0) { return ret; } =20 - if (be32_to_cpu(tpm2_resp->hdr.len) !=3D sizeof(*tpm2_resp) || - be32_to_cpu(tpm2_resp->count) !=3D 2) { + if (be32_to_cpu(tpm2_resp.hdr.len) !=3D sizeof(tpm2_resp) || + be32_to_cpu(tpm2_resp.count) !=3D 2) { DPRINTF("tpm2_resp->hdr.len =3D %u, expected =3D %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 =3D %u, expected =3D %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 =3D MAX(be32_to_cpu(tpm2_resp->value1), - be32_to_cpu(tpm2_resp->value2)); + *buffersize =3D MAX(be32_to_cpu(tpm2_resp.value1), + be32_to_cpu(tpm2_resp.value2)); break; } case TPM_VERSION_UNSPEC: --=20 2.16.0.rc1.1.gef27df75a1