Skip to content

bpo-29738: Fix memory leak in _get_crl_dp#526

Merged
Mariatta merged 2 commits into
python:masterfrom
olivielpeau:fix-issue-29738
Apr 15, 2017
Merged

bpo-29738: Fix memory leak in _get_crl_dp#526
Mariatta merged 2 commits into
python:masterfrom
olivielpeau:fix-issue-29738

Conversation

@olivielpeau

@olivielpeau olivielpeau commented Mar 6, 2017

Copy link
Copy Markdown
Contributor
  • Remove conditional on free of dps, since dps is now allocated for
    all versions of OpenSSL
  • Use sk_DIST_POINT_pop_free instead of sk_DIST_POINT_free since
    the latter doesn't free the individual elements of the stack
  • Remove call to x509_check_ca since it was only used to cache
    the crldp field of the certificate

@olivielpeau
bpo-29738: Fix memory leak in _get_crl_dp
03bdf3c 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Use `sk_DIST_POINT_pop_free` instead of `sk_DIST_POINT_free` since
the latter doesn't free the individual elements of the stack
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
@the-knights-who-say-ni

the-knights-who-say-ni commented Mar 6, 2017

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:

  1. Sign the PSF contributor agreement. The "bugs.python.org username" requested by the form is the "Login name" field in "Your Details" at b.p.o
  2. Wait at least one US business day and then check the "Contributor form received entry under "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  3. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@mention-bot

mention-bot commented Mar 6, 2017

Copy link
Copy Markdown

@olivielpeau, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @Yhg1s, @loewis, @serhiy-storchaka and @Haypo to be potential reviewers.

@olivielpeau

olivielpeau commented Mar 9, 2017

Copy link
Copy Markdown
Contributor Author

@the-knights-who-say-ni : CLA signed

alex
alex approved these changes Mar 24, 2017
View reviewed changes

@alex alex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks correct to me.

I can't remember the rules for merging and backports, so I'm hoping another core dev will actually land :-)

@serhiy-storchaka serhiy-storchaka requested a review from tiran April 2, 2017 13:35
tiran
tiran requested changes Apr 4, 2017
View reviewed changes

@tiran tiran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CRL_DIST_POINTS_free is available in all supported versions of OpenSSL (recent 0.9.8+) and LibreSSL.

Comment thread Modules/_ssl.c Outdated
#if OPENSSL_VERSION_NUMBER < 0x10001000L
sk_DIST_POINT_free(dps);
#endif
sk_DIST_POINT_pop_free(dps, DIST_POINT_free);

@tiran tiran Apr 4, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CRL_DIST_POINTS_free(dps);

@olivielpeau
bpo-29738: Use CRL_DIST_POINTS_free
387f958 
Addresses PR review.

CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
@olivielpeau

olivielpeau commented Apr 5, 2017

Copy link
Copy Markdown
Contributor Author

@tiran Thanks for the review! I've addressed your comment, let me know if this looks good now

@alex alex mentioned this pull request Apr 11, 2017
Closed
@Mariatta Mariatta merged commit 2849cc3 into python:master Apr 15, 2017
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Apr 15, 2017
@olivielpeau @Mariatta
[3.6] bpo-29738: Fix memory leak in _get_crl_dp (pythonGH-526)
e011a80 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Apr 15, 2017
@olivielpeau @Mariatta
[3.5] bpo-29738: Fix memory leak in _get_crl_dp (pythonGH-526)
219011e 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Apr 15, 2017
@olivielpeau @Mariatta
[2.7] bpo-29738: Fix memory leak in _get_crl_dp (pythonGH-526)
f83d3a8 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
Mariatta added a commit that referenced this pull request Apr 15, 2017
@Mariatta
[2.7] bpo-29738: Fix memory leak in _get_crl_dp (GH-526) (GH-1144)
b2b00e0 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
Mariatta added a commit that referenced this pull request Apr 15, 2017
@Mariatta
[3.6] bpo-29738: Fix memory leak in _get_crl_dp (GH-526) (GH-1142)
8e72013 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
Mariatta added a commit that referenced this pull request Apr 15, 2017
@Mariatta
[3.5] bpo-29738: Fix memory leak in _get_crl_dp (GH-526) (GH-1143)
c40740c 
* Remove conditional on free of `dps`, since `dps` is now allocated for
all versions of OpenSSL
* Remove call to `x509_check_ca` since it was only used to cache
the `crldp` field of the certificate
CRL_DIST_POINTS_free is available in all supported versions of OpenSSL
(recent 0.9.8+) and LibreSSL.
(cherry picked from commit 2849cc3)
@Mariatta

Mariatta commented Apr 15, 2017

Copy link
Copy Markdown
Member

Thanks @olivielpeau and congrats for your first contribution to CPython 🎉

@olivielpeau olivielpeau deleted the fix-issue-29738 branch April 17, 2017 23:17
This was referenced May 22, 2017
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alex alex alex approved these changes

@tiran tiran tiran approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@olivielpeau @the-knights-who-say-ni @mention-bot @Mariatta @alex @tiran