Test platform abstraction for memory #6429
Draft
+297
−30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MBEDTLS_MEMORY_BUFFER_ALLOC_C is not set in full, so there is no need to unset it. The calls to unset it were remainders of a time when the option was set in full. This commit extends 24600e8 to work that was in flight at the time. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
added
enhancement
component-platform
gilles-peskine-arm
added
priority-medium
Test that MBEDTLS_PLATFORM_CALLOC_MACRO and MBEDTLS_PLATFORM_FREE_MACRO have the expected effect, namely, to set what mbedtls_calloc() and mbedtls_free() do. The tests are only enabled in a special test configuration where those macros are set to specific functions from the test framework. This special configuration can be activated by setting MBEDTLS_TEST_PLATFORM_MACROS and including "tests/configs/user-config-for-test.h" (typically as MBEDTLS_USER_CONFIG_FILE). This is intended to be the start of a framework that will be generalized to other platform abstraction macros. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
There is no reason to require the platform module just to define replacement macros for platform abstraction functions. This commit removes the requirement for calloc and free, and adds a CI build to ensure this is tested. I expect follow-ups to generalize this to other platform abstraction macros. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
platform-memory-test
branch
from
d42da25
to
04e590d
Compare
Test mbedtls_platform_set_calloc_free(). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The test code needs to know the default functions to restore them after changing them, and the runtime API doesn't provide this information, so the compile-time settings are necessary. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
platform-memory-test
branch
from
04e590d
to
2cf4ff7
Compare
gilles-peskine-arm
added
needs-review
tom-daubney-arm
added
the
historical-reviewing
|
We are now converting older PRs to draft PRs where the following conditions are met: They have not been updated in the last 3 months, and they need work to complete; in this case there are conflicts. |
tom-daubney-arm
added
needs-work
historical-reviewed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test the platform abstraction macros and function that allow customizing
mbedtls_callocandmbedtls_free. Previously, we were testing builds using those features, but not testing that the features had the desired effect.Remove the restriction that
MBEDTLS_PLATFORM_CALLOC_MACROandMBEDTLS_PLATFORM_FREE_MACROrequireMBEDTLS_PLATFORM_C. This restriction was added a long time ago, as part of a batch that made both macro-based replacements and variable-based replacements requireMBEDTLS_PLATFORM_C, but only the variable-based replacements actually needplatform.c. (See #6199 for a bit of discussion on this topic.)I've purposefully kept the scope to just calloc/free. Once we're happy with the general approach, it should be extended to the rest of
platform.h.Changelog: removing a configuration restriction is a new feature, so it gets a changelog entry.
Backport: at least the new tests. Possibly even the removal of the configuration restriction: it doesn't change anything for users who don't try out the newly possible configuration, and I think it'll make our life easier because we'll have an easier time backporting test configurations.