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-37013: Fix the error handling in socket.if_indextoname() #13503

Merged
merged 11 commits into from
Dec 1, 2023
Merged

bpo-37013: Fix the error handling in socket.if_indextoname() #13503

merged 11 commits into from
Dec 1, 2023

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented May 22, 2019 •
edited by bedevere-bot
Loading

@ZackerySpytz
Copy link
Contributor Author

I don't think this needs a unit test, mainly because (unsigned long)-1 can vary across platforms.

matrixise
matrixise approved these changes May 23, 2019
View reviewed changes
Copy link
Member

@matrixise matrixise left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

@zooba
Copy link
Member

zooba commented May 23, 2019

@matrixise I thought we gave you merge permissions? Want to hit the button? :)

serhiy-storchaka
serhiy-storchaka reviewed Jun 1, 2019
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

if_indextoname() takes unsigned int. Therefore highest bits of index can be ignored.

If UINT_MAX < index <= ULONG_MAX -- ignore highest bits.
If ULONG_MAX < index -- OverflowError.

I suggest to handle large index uniformly.

if ((unsigned long)(unsigned int)index != index) {
    PyErr_SetString(OverflowError, "...");
}

@ned-deily
Copy link
Member

This PR seems to have stalled. @serhiy-storchaka, are you waiting for @ZackerySpytz to address your comments?

@serhiy-storchaka
Copy link
Member

Yes, I am waiting for addressing my comments.

And please add tests for socket.if_indextoname() called with 2**32-1, 2**32, 2**64-1, 2**64. OSError and OverflowError should be ignored, but the test should not crash.

@matrixise
Copy link
Member

@matrixise I thought we gave you merge permissions? Want to hit the button? :)

@zoobar yep, with great powers, great responsibilities, but I don't want to crash Python ;-)

@ZackerySpytz could you update your PR with the comments of @serhiy-storchaka ? Thank you

@gpshead gpshead added the needs backport to 3.8 only security fixes label Sep 28, 2019
@csabella
Copy link
Contributor

@ZackerySpytz, please address the code review comments. Thanks!

@csabella
Copy link
Contributor

@serhiy-storchaka, please re-review as I believe your comments have been addressed. Thank you!

@iritkatriel iritkatriel added the needs backport to 3.10 only security fixes label Oct 17, 2021
@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) December 1, 2023 14:43
@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) December 1, 2023 14:44
@serhiy-storchaka serhiy-storchaka merged commit 0daf555 into python:main Dec 1, 2023
35 checks passed
@miss-islington-app
Copy link

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10, 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
@ZackerySpytz @miss-islington
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
85e8ca1 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2023

GH-112597 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 1, 2023
@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2023

GH-112598 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
@ZackerySpytz @miss-islington
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
8ba4168 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Dec 1, 2023
@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2023

GH-112599 is a backport of this pull request to the 3.10 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.10 only security fixes label Dec 1, 2023
@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2023

GH-112600 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
@ZackerySpytz @miss-islington
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
89e2b34 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.9 only security fixes label Dec 1, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2023
@ZackerySpytz @miss-islington
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
1f0c708 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Dec 25, 2023
@miss-islington @ZackerySpytz
[3.12] bpo-37013: Fix the error handling in socket.if_indextoname() (G…
fc7e67f 
…H-13503) (GH-112597)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Dec 25, 2023
@miss-islington @ZackerySpytz
[3.11] bpo-37013: Fix the error handling in socket.if_indextoname() (G…
1cc594d 
…H-13503) (GH-112598)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.8 only security fixes label Dec 25, 2023
@miss-islington-app
Copy link

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 25, 2023
@ZackerySpytz @miss-islington
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
ff396d9 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Dec 25, 2023

GH-113474 is a backport of this pull request to the 3.8 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.8 only security fixes label Dec 25, 2023
@ZackerySpytz ZackerySpytz mannequin mentioned this pull request Dec 25, 2023
Closed
ambv pushed a commit that referenced this pull request Jan 17, 2024
@miss-islington @ZackerySpytz
[3.8] bpo-37013: Fix the error handling in socket.if_indextoname() (G…
2e1156c 
…H-13503) (GH-113474)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
ambv pushed a commit that referenced this pull request Jan 17, 2024
@miss-islington @ZackerySpytz
[3.9] bpo-37013: Fix the error handling in socket.if_indextoname() (G…
a6f73f6 
…H-13503) (GH-112600)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
ambv pushed a commit that referenced this pull request Jan 17, 2024
@miss-islington @ZackerySpytz
[3.10] bpo-37013: Fix the error handling in socket.if_indextoname() (G…
32e7acd 
…H-13503) (GH-112599)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
(cherry picked from commit 0daf555)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@ZackerySpytz @aisk
bpo-37013: Fix the error handling in socket.if_indextoname() (pythonG…
f3e7dcd 
…H-13503)

* Fix a crash when pass UINT_MAX.
* Fix an integer overflow on 64-bit non-Windows platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matrixise matrixise matrixise approved these changes

@vstinner vstinner vstinner approved these changes

@iritkatriel iritkatriel iritkatriel approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

@serhiy-storchaka serhiy-storchaka

@ZackerySpytz ZackerySpytz

Labels
type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet