tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
730 stars 359 forks source link

pack struct breaks abi #531

Closed williamcroberts closed 6 years ago

williamcroberts commented 6 years ago

Commit https://github.com/01org/tpm2-tss/commit/82a2ff9cec3169d33e1ad4433b074d399002f1c0 breaks the abi between tss users and abrmd. Unless all users of the library add pack-struct, then the compiler emits different offsets/padding for the structures members as well as sizeof(struct).

Don't pack public structus shared. This notably hurts when calling from abrmd into the tcti stuffs that use the struct TCTI_SOCKET_CONF.

I'm getting tons of memory issues like this:


==16314== Memcheck, a memory error detector
==16314== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==16314== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==16314== Command: tpm2-abrmd --tcti=socket
==16314== 

** (tpm2-abrmd:16314): WARNING **: BILL CONFIG LOGCB: (nil)

==16314== Thread 2:
==16314== Conditional jump or move depends on uninitialised value(s)
==16314==    at 0x5FBE90E: vfprintf (vfprintf.c:1631)
==16314==    by 0x5FC5898: printf (printf.c:33)
==16314==    by 0x583E6AD: InitSocketTcti (tcti_socket.c:557)
==16314==    by 0x4177FD: tcti_socket_initialize (tcti-socket.c:191)
==16314==    by 0x412BC9: tcti_initialize (tcti.c:54)
==16314==    by 0x4056BD: init_thread_func (tabrmd.c:604)
==16314==    by 0x5AB2BB4: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==16314==    by 0x5D5A6B9: start_thread (pthread_create.c:333)
==16314==    by 0x60773DC: clone (clone.S:109)
``
Note that I have modified tss and abrmd with some print statements to see where the issue is.

Reverting this fixes my problem, revert coming upstream.
tstruk commented 6 years ago

On 09/12/2017 12:04 PM, William Roberts wrote:

Commit 82a2ff9 https://github.com/01org/tpm2-tss/commit/82a2ff9cec3169d33e1ad4433b074d399002f1c0 breaks the abi between tss users and abrmd. Unless all users of the library add pack-struct, then the compiler emits different offsets/padding for the structures members as well as sizeof(struct).

Just created a PR to fix this: https://github.com/01org/tpm2-tss/pull/532 Thanks, -- Tadeusz

flihp commented 6 years ago

Note that the fix from @tstruk #532 still breaks ABI. This is the nature of tracking master though. We will be making more than one change that breaks ABI/API in the run up to a 2.0.0 release.

martinezjavier commented 6 years ago

@tstruk if you merge your commit instead of @williamcroberts', can you please borrow his commit message?

Revert commits without an explanation are really painful since it may not be evident what went wrong with the original commit (in fact any commit without a message makes hard to infer the author's intention behind a change).

tstruk commented 6 years ago

@flihp I have tested the #532 change with tools master.

tstruk commented 6 years ago

@martinezjavier, yes, I'll update the comment. Thanks for the reminder.