-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Fix out-of-bounds heap access in internals.pyx. #47935
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
Conversation
If blknos is empty, we unconditionally access blknos[start] where start is 0. This is an out-of-bounds heap access that can be caught by AddressSanitizer, but it's easy to avoid since we are going to ignore the result anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hawkinsp, nice catch!
If you caught this with ASAN, do you mind explaining how you built + ran with ASAN to find this? (For context, it would be really useful for there to be a snippet in the developer guide about building with ASAN - there some issues especially related to memory leaks where ASAN would be helpful (especially when developing OS's that don't easily support valgrind).
I caught this in a Google-internal build, and I'm actually not sure how to reproduce it outside our environment. In essence I found this using a statically-linked build where a self-built Python interpreter, NumPy, pandas, etc were all instrumented with ASAN. I don't think you could even find this particular bug without instrumenting both NumPy and pandas because the dereference here is of a NumPy array. It may or may not be necessary to be using a statically-linked build, which is a nonstandard configuration of Python and its modules we use internally. While it's probably possible to reproduce the asan report outside our environment with some persistence it doesn't seem easy to do. I'm sorry that's a dissatisfying answer! |
Thanks for explaining - I worried this would be the case. I remember trying to build |
Thanks @hawkinsp. Really happy to have more of these types of fixes found by ASAN |
If blknos is empty, we unconditionally access blknos[start] where start is 0. This is an out-of-bounds heap access that can be caught by AddressSanitizer, but it's easy to avoid since we are going to ignore the result anyway.
If blknos is empty, we unconditionally access blknos[start] where start
is 0. This is an out-of-bounds heap access that can be caught by
AddressSanitizer, but it's easy to avoid since
we are going to ignore the result anyway.
No new tests, because this does not change any observable behaviors.