Skip to content

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 16, 2025

  • Don't call close() if the file descriptor is negative.
  • If close() fails, chain the existing exception.

* Don't call close() if the file descriptor is negative.
* If close() fails, chain the existing exception.
@vstinner
Copy link
Member Author

cc @sergey-miryanov @pablogsal

@vstinner
Copy link
Member Author

Example to trigger #130052 (comment) bug.

Apply the patch:

diff --git a/Modules/_testexternalinspection.c b/Modules/_testexternalinspection.c
index e90cfb9132b..dfb874c2f9a 100644
--- a/Modules/_testexternalinspection.c
+++ b/Modules/_testexternalinspection.c
@@ -335,6 +335,7 @@ search_map_for_section(pid_t pid, const char* secname, const char* map)
     uintptr_t result = 0;
     void* file_memory = NULL;
 
+elf_file[0] = 'X';
     int fd = open(elf_file, O_RDONLY);
     if (fd < 0) {
         PyErr_SetFromErrno(PyExc_OSError);

Run the test:

$ ./python -m test -v test_external_inspection -v
(...)
0:00:00 load avg: 0.91 [1/1] test_external_inspection
test_async_gather_remote_stack_trace (test.test_external_inspection.TestGetStackTrace.test_async_gather_remote_stack_trace) ...

python: Objects/call.c:342: _PyObject_Call: Assertion `!_PyErr_Occurred(tstate)' failed.
Fatal Python error: Aborted

Current thread 0x00007fc09e64d780 [python] (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_external_inspection.py", line 271 in test_async_gather_remote_stack_trace
  (...)

Extension modules: _testinternalcapi, _testexternalinspection (total: 2)
Abandon (core dumped)

@pablogsal
Copy link
Member

LGTM thanks a lot @vstinner

@vstinner
Copy link
Member Author

There is another bug in find_map_start_address(), it fails to retrieve the ELF filename in some cases. But I chose to focus on the first most urgent bug. The bug is hard is reproduce.

@vstinner vstinner enabled auto-merge (squash) April 16, 2025 13:55
@vstinner vstinner merged commit 014c7f9 into python:main Apr 16, 2025
53 checks passed
@vstinner vstinner deleted the search_map branch April 16, 2025 13:56
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 16, 2025
…H-132594)

* Don't call close() if the file descriptor is negative.
* If close() fails, chain the existing exception.
(cherry picked from commit 014c7f9)

Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Apr 16, 2025

GH-132598 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Apr 16, 2025
vstinner added a commit that referenced this pull request Apr 16, 2025
) (#132598)

gh-130052: Fix search_map_for_section() error handling (GH-132594)

* Don't call close() if the file descriptor is negative.
* If close() fails, chain the existing exception.
(cherry picked from commit 014c7f9)

Co-authored-by: Victor Stinner <vstinner@python.org>
@sergey-miryanov
Copy link
Contributor

@vstinner thanks for the fix! Do you want me to look at the second bug?

@vstinner
Copy link
Member Author

Do you want me to look at the second bug?

If you want. Sometimes, find_map_start_address() fails to parse the ELF filename and so open() fails. For example, the filename starts with spaces and ends with a newline. I don't know how to reproduce the bug in a reliable way.

@sergey-miryanov
Copy link
Contributor

If you want. Sometimes, find_map_start_address() fails to parse the ELF filename and so open() fails. For example, the filename starts with spaces and ends with a newline. I don't know how to reproduce the bug in a reliable way.

It seems that @pablogsal fixed this, at least he handle \n in the latest code:

while (fgets(line + linelen, linesz - linelen, maps_file) != NULL) {
linelen = strlen(line);
if (line[linelen - 1] != '\n') {
// Read a partial line: realloc and keep reading where we left off.
// Note that even the last line will be terminated by a newline.
linesz *= 2;
char *biggerline = PyMem_Realloc(line, linesz);
if (!biggerline) {
PyMem_Free(line);
fclose(maps_file);
PyErr_NoMemory();
return 0;
}
line = biggerline;
continue;
}
.

I tried to reproduce, but python doesn't run at all.

root@s512:~/sources/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111/2222222222222222222-2222222222222222222-2222222222222222222-2222222222222222222-2222222222222222222/33333333333333333333333333333-33333333333333333333333333333-33333333333333333333333333333/cpython# ./python -m test -v test_external_inspection 
Exception ignored while running getpath:
Traceback (most recent call last):
  File "<frozen getpath>", line 361, in <module>
SystemError: failed to join paths
Fatal Python error: error evaluating path
Python runtime state: core initialized

Stack (most recent call first):
  <no Python frame>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants