-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[C++][Acero] ASAN reports heap buffer overflow in arrow::compute::KeyCompare::CompareBinaryColumnToRow
#39577
Comments
The cause seems to be that, the default buffer alignment (64b) doesn’t guarantee tail bytes safety when doing by-word operation for long fixed size types. Did some debugging, I found for this particular case, an encoded row took 19b, and there were 165 rows. They took 19b * 165 = 3135b, so 3136b is the actual size aligned by 64b. The last row access started at byte 3116 for 3 words (24b), which eventually exceeded the size 3136 buffer boundary by 4b. I’m working on a fix. |
Take |
…eBinaryColumnToRow` (#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment #39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: #39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
…CompareBinaryColumnToRow` (apache#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment apache#39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: apache#39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
…CompareBinaryColumnToRow` (apache#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment apache#39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: apache#39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
arrow::compute::KeyCompare::CompareBinaryColumnToRow
…CompareBinaryColumnToRow` (apache#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment apache#39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: apache#39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
…eBinaryColumnToRow` (#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment #39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: #39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
…CompareBinaryColumnToRow` (apache#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment apache#39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: apache#39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
…CompareBinaryColumnToRow` (apache#39606) ### Rationale for this change Default buffer alignment (64b) doesn't guarantee the safety of tail-word access in `KeyCompare::CompareBinaryColumnToRow`. Comment apache#39577 (comment) is a concrete example. ### What changes are included in this PR? Make `KeyCompare::CompareBinaryColumnToRow` tail-word safe. ### Are these changes tested? UT included. ### Are there any user-facing changes? No. * Closes: apache#39577 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
Describe the bug, including details regarding any error messages, version, and platform.
Hardware
Apple M1 Pro
OS
macOS Sonoma 14.1.1 (23B81)
Version
3cc04f1
Reproduce
Change test
HashJoin.Random
code to run more times, e.g.1000
:arrow/cpp/src/arrow/acero/hash_join_node_test.cc
Line 981 in 3cc04f1
Build with ASAN enabled and all allocators disabled:
Run specific test:
Result:
Component(s)
C++
The text was updated successfully, but these errors were encountered: