Fix union initialization in PSA operations for GCC 15: new test helpers #136
Conversation
gilles-peskine-arm
added
needs-ci
For each multipart or interruptible operation, define an initializer
function that simulates the minimum that `my_op_t op = {0}` guarantees in C.
That is, initialize most fields to 0, but set the fields that are unions to
a nonzero value. This simulates platforms where initializing a union to
`{0}` only initializes the first member, and thus reading from another
member can yield a nonzero value. In our operation structures, the union's
first member is an unused `dummy`, and the other members are
driver-specific, so we just make the whole union nonzero and this has to be
good enough for the setup functions in the core to cope.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This fixes -Wmissing-field-initializers complaints from Clang <=3.x. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix the build against development. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
union-initialization-gcc15-framework-preliminaries
branch
from
3ca1d6a to
92f2203
Compare
gilles-peskine-arm
added
needs-review
| #ifndef PSA_CRYPTO_TEST_DRIVERS_TEST_DRIVER_COMMON_H | ||
| #define PSA_CRYPTO_TEST_DRIVERS_TEST_DRIVER_COMMON_H | ||
|
|
||
| #include "mbedtls/build_info.h" |
There was a problem hiding this comment.
Note: currently this is already included by all users before deciding whether to include this file. But it doesn't hurt now and might be helpful later, so no objection.
| @@ -145,6 +145,35 @@ const char *mbedtls_test_helper_is_psa_leaking(void); | |||
| while (0) | |||
|
|
|||
|
|
|||
| /** Initializer that doesn't set the embedded union to zero. | |||
There was a problem hiding this comment.
Note: I grepped include/psa' for union {, found 15 occurrences, and verify that all of them chain to a structure that's in the list below - which has 3 extras entries that don't have a union yet. Some structures have more than one union, directly or indirectly, here the larger number of matches for union {` then structures below.
github-project-automation
bot
moved this from In Development
to Has Approval
in Roadmap pull requests (new board)
mpg
added
approved
|
@gilles-peskine-arm This is fully approved and has a green CI (Open), so I'm inclined to merge it. I see the description has "TODO" for the crypto/TLS/3.6 PRs but I don't think we need to wait for that in order to merge this one, considering it has a green CI already? |
|
This just needs submodule updates in the consuming branches, then it can start being consumed. Merging it would be good (and thanks for the reviews). The CI result is three months old though, so I'll just merge |
…ramework-preliminaries
github-project-automation
bot
moved this from Has Approval
to Done
in Roadmap pull requests (new board)
New test helpers that will be used for the fix of Mbed-TLS/mbedtls#9814.
This just adds new functions that can be consumed by new test code (Mbed-TLS/mbedtls#9955 — I'm planning some history rewrite, but no code changes except to handle CI and review feedback). There will be a follow-up in the framework that adds new checks (bd33fb4) that will only pass once the library is fixed. (At the moment, the new checks fail after
abort()for some operations, because in abort functions, we consistently clean up core metadata and sensitive data in operation objects, but we sometimes leave driver-specific metadata behind, which makes the setup-time is-all-zero check fail when an operation object is reused after abort/finish.)PR checklist