GH-49966: [C++] Detect different endianness between IPC file and stream in IPC file fuzzer - #49968
Conversation
ece9732 to
7dbefac
Compare
|
@github-actions crossbow submit -g cpp |
|
@kiszk Is it possible for you to run the new unit test |
This comment was marked as outdated.
This comment was marked as outdated.
|
Thank you. I will do it next week since I am taking a vacation this week. |
|
Also @paleolimbot FYI |
7dbefac to
b26a93d
Compare
|
It looks like the Valgrind errors are related, I'll take a look. |
There was a problem hiding this comment.
Thank you! These changes seem reasonable pending CI.
I'm wondering if I need to update anything in nanoarrow's reader to cover this case. I don't think so because we always byte swap to native endian on read (i.e., no option to skip this) but I may be misunderstanding the failure.
I don't think there's anything to do, because the problem was in the fuzz target, not in the IPC reader itself. Valid IPC files should have the same schema in the file footer and in the embedded IPC stream. |
…d stream in IPC file fuzzer
b26a93d to
86203c1
Compare
@kiszk Ping :) |
|
@github-actions crossbow submit -g cpp |
|
Revision: 86203c1 Submitted crossbow builds: ursacomputing/crossbow @ actions-8f178ec382 |
8ba9a96 to
4c61110
Compare
|
@github-actions crossbow submit test-conda-cpp-valgrind |
|
Revision: 4c61110 Submitted crossbow builds: ursacomputing/crossbow @ actions-5600ef63e7
|
|
I managed to cross-compile Arrow for s390x and then run the unit tests under emulation. The new |
|
After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit de5b933. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
|
@pitrou Sorry, I was busy after a vacation. While it was merged, I will check it this weekend. |
|
@pitrou It works well on big-endian machine 🎉 Thank you very much for taking care of big-endian. % |
Rationale for this change
In the IPC file format, the IPC file footer has a copy of the embedded IPC stream's schema. However, the two copies may be different in case of an invalid/corrupted IPC file. The fuzzer would then fail with differing contents between IPC file and stream, as only one of them would have undergone endianness swapping.
Issue found by OSS-Fuzz: https://issues.oss-fuzz.com/issues/506111650
What changes are included in this PR?
ipc::ReadStatsthat reflects the original endianness of the IPC schema (before the endianness was normalized to native)Are these changes tested?
Yes, by additional unit tests and additional fuzz regression file.
Are there any user-facing changes?
No.