Skip to content

Fix memory leak in tls_parse_ctos_psk()#25643

Closed
ndossche wants to merge 1 commit into
openssl:masterfrom
ndossche:extensions_srvr-1
Closed

Fix memory leak in tls_parse_ctos_psk()#25643
ndossche wants to merge 1 commit into
openssl:masterfrom
ndossche:extensions_srvr-1

Conversation

@ndossche
Copy link
Copy Markdown
Contributor

@ndossche ndossche commented Oct 9, 2024 •
edited
Loading

sess is not NULL at this point, and is freed on the success path, but not on the error path. Fix this by going to the err label such that SSL_SESSION_free(sess) is called.

CLA: trivial

Disclaimer: this bug was found by an experimental static analyzer I'm working on. As such, I could be wrong because even though I manually read the code as well.

@ndossche
Fix memory leak in tls_parse_ctos_psk()
f672ad3 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial
@t8m
Copy link
Copy Markdown
Member

t8m commented Oct 9, 2024

OK with CLA: trivial

@t8m t8m added branch: master Applies to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug cla: trivial One of the commits is marked as 'CLA: trivial' branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) tests: exempted The PR is exempt from requirements for testing branch: 3.2 Applies to openssl-3.2 (EOL) branch: 3.3 Applies to openssl-3.3 (EOL) branch: 3.4 Applies to openssl-3.4 labels Oct 9, 2024
Sashan
Sashan approved these changes Oct 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@Sashan Sashan left a comment

Choose a reason for hiding this comment

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

looks good to me thanks.

tom-cosgrove-arm
tom-cosgrove-arm approved these changes Oct 10, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@tom-cosgrove-arm tom-cosgrove-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

@tom-cosgrove-arm tom-cosgrove-arm added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 10, 2024
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 11, 2024
@openssl-machine
Copy link
Copy Markdown
Collaborator

openssl-machine commented Oct 11, 2024

This pull request is ready to merge

@t8m
Copy link
Copy Markdown
Member

t8m commented Oct 11, 2024

Merged to all the active branches. Thank you for your contribution.

@t8m t8m closed this Oct 11, 2024
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
e11fa05 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)

(cherry picked from commit b2474b2)
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
c58b6fd 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)

(cherry picked from commit b2474b2)
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
b2474b2 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
93b9ba0 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)

(cherry picked from commit b2474b2)
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
32f06c5 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)

(cherry picked from commit b2474b2)
openssl-machine pushed a commit that referenced this pull request Oct 11, 2024
@ndossche @t8m
Fix memory leak in tls_parse_ctos_psk()
b00a6f0 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #25643)

(cherry picked from commit b2474b2)
eclipse-oniro-oh-bot pushed a commit to eclipse-oniro-mirrors/third_party_openssl that referenced this pull request Oct 18, 2024
@ndossche @lan1120
Fix memory leak in tls_parse_ctos_psk()
cdfeb90 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl/openssl#25643)

Signed-off-by: lanming <lanming@huawei.com>
eclipse-oniro-oh-bot pushed a commit to eclipse-oniro-mirrors/third_party_openssl that referenced this pull request Oct 21, 2024
@ndossche @lan1120
Fix memory leak in tls_parse_ctos_psk()
3a0fc06 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl/openssl#25643)

Signed-off-by: lanming <lanming@huawei.com>
coolshrid pushed a commit to coolshrid/openssl that referenced this pull request Nov 9, 2024
@ndossche @coolshrid
Fix memory leak in tls_parse_ctos_psk()
652d897 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

CLA: trivial

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#25643)
@xnox xnox mentioned this pull request Nov 11, 2024
Closed
Maxlsc added a commit to Maxlsc/Tongsuo that referenced this pull request May 7, 2026
@Maxlsc
Fix memory leak in tls_parse_ctos_psk()
b27c48d 
`sess` is not NULL at this point, and is freed on the success path, but
not on the error path. Fix this by going to the `err` label such that
`SSL_SESSION_free(sess)` is called.

Merged from openssl/openssl#25643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sashan Sashan Sashan approved these changes

@t8m t8m t8m approved these changes

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

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) branch: 3.2 Applies to openssl-3.2 (EOL) branch: 3.3 Applies to openssl-3.3 (EOL) branch: 3.4 Applies to openssl-3.4 cla: trivial One of the commits is marked as 'CLA: trivial' tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ndossche @t8m @openssl-machine @Sashan @tom-cosgrove-arm