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.

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

Assertion failure for socket with too large default timeout (larger than INT_MAX) #126876

Open
devdanzin opened this issue Nov 15, 2024 · 4 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@devdanzin
Copy link
Contributor

devdanzin commented Nov 15, 2024

Crash report

What happened?

A debug build will abort when calling socket._fallback_socketpair() after a call to socket.setdefaulttimeout with too high a value:

python -c "import socket; socket.setdefaulttimeout(2**31) ; socket._fallback_socketpair()"
python: ./Modules/socketmodule.c:819: internal_select: Assertion `ms <= INT_MAX' failed.
Aborted (core dumped)

Found using fusil by @vstinner.

CPython versions tested on:

3.12, 3.13, 3.14, CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.13.0+ (heads/3.13:7be8743, Nov 15 2024, 15:20:16) [GCC 13.2.0]

Linked PRs

@devdanzin devdanzin added the type-crash A hard crash of the interpreter, possibly with a core dump label Nov 15, 2024
@picnixz picnixz added extension-modules C modules in the Modules dir 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Nov 15, 2024
@sobolevn
Copy link
Member

Here's my idea:

diff --git Modules/socketmodule.c Modules/socketmodule.c
index 2764bd6e2b2..4c0aac92a76 100644
--- Modules/socketmodule.c
+++ Modules/socketmodule.c
@@ -810,7 +810,7 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval,
 
     /* s->sock_timeout is in seconds, timeout in ms */
     ms = _PyTime_AsMilliseconds(interval, _PyTime_ROUND_CEILING);
-    assert(ms <= INT_MAX);
+    assert(ms <= PyTime_MAX);
 
     /* On some OSes, typically BSD-based ones, the timeout parameter of the
        poll() syscall, when negative, must be exactly INFTIM, where defined,
                                               

After that:

» ./python.exe -c "import socket; socket.setdefaulttimeout(2**31) ; socket._fallback_socketpair()"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import socket; socket.setdefaulttimeout(2**31) ; socket._fallback_socketpair()
                                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/sobolev/Desktop/cpython2/Lib/socket.py", line 633, in _fallback_socketpair
    ssock, _ = lsock.accept()
               ~~~~~~~~~~~~^^
  File "/Users/sobolev/Desktop/cpython2/Lib/socket.py", line 298, in accept
    fd, addr = self._accept()
               ~~~~~~~~~~~~^^
TimeoutError: timed out
                       

@sobolevn
Copy link
Member

If others agree with me - I can open a PR :)

@vstinner
Copy link
Member

-    assert(ms <= INT_MAX);
+    assert(ms <= PyTime_MAX);

This change is wrong: poll() last argument type is int. A better fix would be:

diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index 2764bd6e2b2..0ba5b4af6bc 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -810,7 +810,9 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval,
 
     /* s->sock_timeout is in seconds, timeout in ms */
     ms = _PyTime_AsMilliseconds(interval, _PyTime_ROUND_CEILING);
-    assert(ms <= INT_MAX);
+    if (ms > INT_MAX) {
+        ms = INT_MAX;
+    }
 
     /* On some OSes, typically BSD-based ones, the timeout parameter of the
        poll() syscall, when negative, must be exactly INFTIM, where defined,

vstinner added a commit to vstinner/cpython that referenced this issue Nov 18, 2024
If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
@vstinner
Copy link
Member

I propose PR gh-126968 to fix the issue.

@vstinner vstinner changed the title Assertion failure if for socket._fallback_socketpair with too large default timeout Assertion failure if for socket with too large default timeout (larger than INT_MAX) Nov 18, 2024
@vstinner vstinner changed the title Assertion failure if for socket with too large default timeout (larger than INT_MAX) Assertion failure for socket with too large default timeout (larger than INT_MAX) Nov 18, 2024
vstinner added a commit that referenced this issue Nov 19, 2024
If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 19, 2024
…onGH-126968)

If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
(cherry picked from commit b3687ad)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 19, 2024
…onGH-126968)

If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
(cherry picked from commit b3687ad)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit to vstinner/cpython that referenced this issue Dec 2, 2024
vstinner added a commit to miss-islington/cpython that referenced this issue Dec 2, 2024
vstinner added a commit to miss-islington/cpython that referenced this issue Dec 2, 2024
vstinner added a commit that referenced this issue Dec 2, 2024
…126968) (#127002)

gh-126876: Fix socket internal_select() for large timeout (GH-126968)

If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
(cherry picked from commit b3687ad)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit that referenced this issue Dec 3, 2024
…126968) (#127003)

gh-126876: Fix socket internal_select() for large timeout (GH-126968)

If the timeout is larger than INT_MAX, replace it with INT_MAX, in
the poll() code path.

Add an unit test.
(cherry picked from commit b3687ad)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

4 participants