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

bpo-29738: Fix memory leak in _get_crl_dp #526

Merged
merged 2 commits into from
Apr 15, 2017
Merged

bpo-29738: Fix memory leak in _get_crl_dp #526

merged 2 commits into from
Apr 15, 2017

Conversation

olivielpeau
Copy link
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
Copy link

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
Copy link

@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
Copy link
Contributor Author

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

alex
alex approved these changes Mar 24, 2017
View reviewed changes
Copy link
Member

@alex alex left a comment

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
Copy link
Member

@tiran tiran left a comment

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.

Modules/_ssl.c Outdated
#if OPENSSL_VERSION_NUMBER < 0x10001000L
sk_DIST_POINT_free(dps);
#endif
sk_DIST_POINT_pop_free(dps, DIST_POINT_free);
Copy link
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
Copy link
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
Copy link
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