Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set psk to NULL in ssl_psk_remove #9241

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Set psk to NULL in ssl_psk_remove #9241

merged 1 commit into from
Jun 12, 2024

Conversation

lhuang04
Copy link
Contributor

@lhuang04 lhuang04 commented Jun 10, 2024 •
edited by tom-cosgrove-arm
Loading

Summary:
set the psk to NULL after it is released. ssl_remove_psk will freessl->handshake->psk. If we don't set it to NULL. It may be used or free again in other conditions.

Test Plan:

Reviewers:

Subscribers: [email protected]

Tasks:

Tags:

Description

Address an issue that psk is not set to NULL after it is released.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@lhuang04
set psk to null in ssl_psk_remove
54adeab 
Summary:
set the psk to null after it is released.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Signed-off-by: lhuang04 <[email protected]>
@tom-cosgrove-arm tom-cosgrove-arm 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 component-tls labels Jun 11, 2024
@tom-cosgrove-arm
Copy link
Contributor

Could you please raise backports against the mbedtls-2.28 and mbedtls-3.6 branches as well, please?

@tom-cosgrove-arm tom-cosgrove-arm added size-s Estimated task size: small (~2d) priority-medium Medium priority - this can be reviewed as time permits labels Jun 11, 2024
gilles-peskine-arm
gilles-peskine-arm approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

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

Choose a reason for hiding this comment

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

LGTM — thanks, setting a pointer variable to null after it's freed is a good hygiene rule.

Can you please make backports to the mbedtls-3.6 and mbedtls-2.28 branches?

@gilles-peskine-arm gilles-peskine-arm added needs-backports Backports are missing or are pending review and approval. approved Design and code approved - may be waiting for CI or backports and removed 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 labels Jun 11, 2024
lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove-backport to 2.28
d465da7 
Summary:
Back port [PR 9241](Mbed-TLS#9241) to
2.28 branch

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove -backpor to 3.6
c01f367 
Summary:
Backport [PR 9241](Mbed-TLS#9241) to 3.6
branch.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove -backpor to 3.6
ec7a97d 
Summary:
Backport [PR 9241](Mbed-TLS#9241) to 3.6
branch.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@lhuang04 lhuang04 mentioned this pull request Jun 11, 2024
Merged
4 tasks
lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove-backport to 2.28
841c2b1 
Summary:
Back port [PR 9241](Mbed-TLS#9241) to
2.28 branch

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@lhuang04 lhuang04 mentioned this pull request Jun 11, 2024
Merged
4 tasks
@lhuang04
Copy link
Contributor Author

Backport PR 9245 for 3.6 branch, and PR 9246 for 2.8 branch.

lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove-backport to 2.28
0a2dd6d 
Summary:
Back port [PR 9241](Mbed-TLS#9241) to
2.28 branch

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Signed-off-by: lhuang04 <[email protected]>
lhuang04 added a commit to lhuang04/mbedtls that referenced this pull request Jun 11, 2024
@lhuang04
Set psk to NULL in ssl_psk_remove -backpor to 3.6
6d4d94f 
Summary:
Backport [PR 9241](Mbed-TLS#9241) to 3.6
branch.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Signed-off-by: lhuang04 <[email protected]>
@tom-cosgrove-arm tom-cosgrove-arm added this pull request to the merge queue Jun 12, 2024
Merged via the queue into Mbed-TLS:development with commit a0cfe01 Jun 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tom-cosgrove-arm tom-cosgrove-arm tom-cosgrove-arm approved these changes

@gilles-peskine-arm gilles-peskine-arm gilles-peskine-arm approved these changes

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports component-tls needs-backports Backports are missing or are pending review and approval. priority-medium Medium priority - this can be reviewed as time permits size-s Estimated task size: small (~2d)
Projects
Non-roadmap pull requests
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lhuang04 @tom-cosgrove-arm @gilles-peskine-arm