Commit cc1b6c55 authored by Marc-André Lureau's avatar Marc-André Lureau Committed by Stefan Berger
Browse files

tpm: fix alignment issues 



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: default avatarPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: default avatarMarc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: default avatarStefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: default avatarStefan Berger <stefanb@linux.vnet.ibm.com>
parent 30270587

hw/tpm/tpm_emulator.c

+6 −8
Original line number Diff line number Diff line
@@ -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;

hw/tpm/tpm_passthrough.c

+2 −4
Original line number Diff line number Diff line
@@ -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:

hw/tpm/tpm_util.c

+34 −41
Original line number Diff line number Diff line
@@ -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:

hw/tpm/tpm_util.h

+16 −1
Original line number Diff line number Diff line
@@ -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,