Repository navigation
gh-148178: Validate remote debug offset tables on load - #148187
pablogsal merged 10 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Also add Linux poison tests that corrupt both the main and asyncio offset tables and assert that RemoteUnwinder fails with an invalid-offset error instead of walking into bad accesses or misleading late failures.
Where are they?
I'm also wondering about char[] buffers and GET_MEMBER? I'm thinking about alignment of offsets for a given type. Perhaps there's a risk of undefined behaviour there as well. Do we care about it?
Sorry, something went wrong.
I decided to remove the tests because there were stupidly long, verbose (as I am basically parsing elf by hand in Python), only worked on Linux and has reduced value (just one way to trigger it not all of them) and the Pr is already quite big but I didn't update the commit message You can see them in the diff here https://github.com/python/cpython/compare/f12f6c4f0467837e25a445339f2e5a24388a317d..b8dcd8fa2b3be846b1dad6d2afb1dccd968789b3 |
Sorry, something went wrong.
I don't think so. But if you have a specific suggestion code suggestion happy to consider it ;) |
Sorry, something went wrong.
Treat the debug offset tables read from a target process as untrusted input and validate them before the unwinder uses any reported sizes or offsets. Add a shared validator in debug_offsets_validation.h and run it once when _Py_DebugOffsets is loaded and once when AsyncioDebug is loaded. The checks cover section sizes used for fixed local buffers and every offset that is later dereferenced against a local buffer or local object view. This keeps the bounds checks out of the sampling hot path while rejecting malformed tables up front.
|
I think the easiest would be: -#define GET_MEMBER(type, obj, offset) (*(type*)((char*)(obj) + (offset)))
+#define GET_MEMBER(type, obj, offset) (*(const type *)memcpy(&(type){0}, (const char *)(obj) + (offset), sizeof(type)))I did not test it, but looks the least disruptive. I was only able to confirm the issue w/ UBSan You already do use memcpy:) I'd love to submit a PR but I think it should be based on yours, not the main branch. I'd definitely include some fail-fast in validate_field() and add field_alignment next to field_size. If you're ok, should be under gh-148178? |
Sorry, something went wrong.
Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
|
@maurycy you can create a pr targetting this branch in my fork if you want |
Sorry, something went wrong.
|
Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
|
Sorry, @pablogsal, I could not cleanly backport this to 3.14 due to a conflict. cherry_picker 289fd2c97a7e5aecb8b69f94f5e838ccfeee7e67 3.14 |
Sorry, something went wrong.
|
GH-148577 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
…148187) Treat the debug offset tables read from a target process as untrusted input and validate them before the unwinder uses any reported sizes or offsets. Add a shared validator in debug_offsets_validation.h and run it once when _Py_DebugOffsets is loaded and once when AsyncioDebug is loaded. The checks cover section sizes used for fixed local buffers and every offset that is later dereferenced against a local buffer or local object view. This keeps the bounds checks out of the sampling hot path while rejecting malformed tables up front.
) (#148577) Treat the debug offset tables read from a target process as untrusted input and validate them before the unwinder uses any reported sizes or offsets. Add a shared validator in debug_offsets_validation.h and run it once when _Py_DebugOffsets is loaded and once when AsyncioDebug is loaded. The checks cover section sizes used for fixed local buffers and every offset that is later dereferenced against a local buffer or local object view. This keeps the bounds checks out of the sampling hot path while rejecting malformed tables up front. (cherry picked from commit 289fd2c)
Treat the debug offset tables read from a target process as untrusted input and validate them before the unwinder uses any reported sizes or offsets.
Add a shared validator in debug_offsets_validation.h and run it once when _Py_DebugOffsets is loaded and once when AsyncioDebug is loaded. The checks cover section sizes used for fixed local buffers and every offset that is later dereferenced against a local buffer or local object view. This keeps the bounds checks out of the sampling hot path while rejecting malformed tables up front.