Skip to content

Fix resource leak in populate transform - #10926

Open
bjwtaylor wants to merge 3 commits into
Mbed-TLS:developmentfrom
bjwtaylor:tl13-transform-resource-leak
Open

bjwtaylor wants to merge 3 commits into
Mbed-TLS:developmentfrom
bjwtaylor:tl13-transform-resource-leak

Conversation

@bjwtaylor

@bjwtaylor bjwtaylor commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fix resource leak in populate transform fixes #10917

PR checklist

@bjwtaylor bjwtaylor changed the title Tl13 transform resource leak Fix resource leak in populate transform Aug 20, 2026
@bjwtaylor
bjwtaylor force-pushed the tl13-transform-resource-leak branch from 27c787f to 2a37227 Compare August 25, 2026 12:36
@bjwtaylor
bjwtaylor marked this pull request as ready for review August 26, 2026 07:18
@bjwtaylor bjwtaylor added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Aug 26, 2026
@gilles-peskine-arm
gilles-peskine-arm self-requested a review August 26, 2026 07:37

@gilles-peskine-arm gilles-peskine-arm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except the test assertion isn't precise enough

Comment thread tests/suites/test_suite_ssl.function Outdated
Comment thread tests/suites/test_suite_ssl.function Outdated
Comment thread tests/suites/test_suite_ssl.function
Comment thread tests/suites/test_suite_ssl.function

@gilles-peskine-arm gilles-peskine-arm left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM at aeaab79

#endif

PSA_INIT();
TEST_CALLOC(keys, SSL_TEST_MAX_VOLATILE_KEYS + 1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why you went back to a dynamic allocation, even though the size is known at compile time?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason one of the compilers was complaining about it in one of the tests and this seemed to resolve the issue. I would link the test, though I don't think the CI has kept it.

@gilles-peskine-arm gilles-peskine-arm added needs-backports Backports are missing or are pending review and approval. and removed needs-work labels Aug 27, 2026
@gilles-peskine-arm gilles-peskine-arm moved this to 1.3 release in Mbed TLS Epics Aug 27, 2026
@bjwtaylor

bjwtaylor commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor Author

@gilles-peskine-arm are you happy for me to squash this into the original 3 commits to assist the backports?

@gilles-peskine-arm

Copy link
Copy Markdown
Contributor

Ok to rewrite the history to (test, fix, changelog).

Ben Taylor added 3 commits September 23, 2026 08:30
Signed-off-by: Ben Taylor <ben.taylor@linaro.org>
Signed-off-by: Ben Taylor <ben.taylor@linaro.org>
Signed-off-by: Ben Taylor <ben.taylor@linaro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)

Projects

Status: 1.3 release

Development

Successfully merging this pull request may close these issues.

TLS 1.3 transform setup can leave an imported traffic key undisposed on failure

2 participants