gh-111178: fix UBSan failures in `Modules/_ssl.c` by picnixz · Pull Request #130719 · python/cpython · GitHub
Skip to content

gh-111178: fix UBSan failures in Modules/_ssl.c - #130719

Merged
encukou merged 10 commits into
python:mainfrom
picnixz:fix/ubsan/ssl-111178
Mar 17, 2025
Merged

gh-111178: fix UBSan failures in Modules/_ssl.c#130719
encukou merged 10 commits into
python:mainfrom
picnixz:fix/ubsan/ssl-111178

Conversation

@picnixz

@picnixz picnixz commented Mar 1, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-app bedevere-app Bot mentioned this pull request Mar 1, 2025
Closed
@picnixz
picnixz requested a review from encukou March 2, 2025 12:27
@ZeroIntensity

ZeroIntensity commented Mar 3, 2025

Copy link
Copy Markdown
Member

@encukou

encukou commented Mar 3, 2025

Copy link
Copy Markdown
Member

I don't think we should be backporting all of them preemptively. But if you get conflicts, it's probably best to backport the UB PR first.

@picnixz

picnixz commented Mar 3, 2025

Copy link
Copy Markdown
Member Author

I haven't backported the changes which caused conflicts on a recent PR but I resolved them manually. I'm afraid that the backports of UB PRs would actually be harder because #128447 was not backported.

encukou
encukou reviewed Mar 3, 2025
View reviewed changes
Comment thread Modules/_ssl.c Outdated
vstinner
vstinner reviewed Mar 7, 2025
View reviewed changes
Comment thread Modules/_ssl.c Outdated
Comment thread Modules/_ssl.c Outdated
Comment thread Modules/_ssl.c Outdated
Comment thread Modules/_ssl/cert.c

@vstinner vstinner Mar 7, 2025

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.

I don't thinkt that this change is useful.

@picnixz picnixz Mar 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I myself did the change and I at that time put an underscore. But considering I used the convention "objectname+_CAST" for the series of PR and that I amended some changes I've done recently I think we can refactor this one as well (if the code is however not by me and/or is older, I can revert that change)

@picnixz picnixz Mar 16, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note: it's something I've changed last month, and I thought I fixed all the UBSan issues in the SSL module but this wasn't the case. I can revert it if you don't want this additional diff.

@picnixz
picnixz requested review from encukou and vstinner March 16, 2025 16:52
picnixz
picnixz commented Mar 16, 2025
View reviewed changes
Comment thread Modules/_ssl.c Outdated
@encukou
encukou merged commit e708ac8 into python:main Mar 17, 2025
@picnixz
picnixz deleted the fix/ubsan/ssl-111178 branch March 17, 2025 15:50
plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
@picnixz @plashchynski
pythongh-111178: fix UBSan failures in Modules/_ssl.c (pythonGH-130719
319039c 
)

* fix UBSan failures for `PySSLContext`, `PySSLSocket`, `PySSLMemoryBIO`, `PySSLSession`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou approved these changes
@vstinner vstinner Awaiting requested review from vstinner

Assignees

No one assigned

Labels

skip news topic-SSL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@picnixz @ZeroIntensity @encukou @vstinner