Skip to content
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

[sw] SPI PAGE_PROGRAM command can trigger segfault in bootstrap #19132

Closed
dmcardle opened this issue Jul 6, 2023 · 2 comments
Closed

[sw] SPI PAGE_PROGRAM command can trigger segfault in bootstrap #19132

dmcardle opened this issue Jul 6, 2023 · 2 comments
Assignees

Comments

@dmcardle
Copy link
Contributor

dmcardle commented Jul 6, 2023

Description

I have been toying with a fuzzer that generates SPI commands in #18929, which discovered a segfault produced by a PAGE_PROGRAM command with a payload_byte_count. I converted this input to a unit test, patched it onto the master branch, and verified it still produces the segfault in #19131.

It looks like we're not properly bounds-checking in bootstrap_page_program().

$ ./bazelisk.sh test --config=asan --test_output=streamed //sw/device/silicon_creator/rom:bootstrap_unittest
[...]
AddressSanitizer:DEADLYSIGNAL
=================================================================
==12==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7f29cfdfc03b bp 0x7ffc950a5ae0 sp 0x7ffc950a5a60 T0)
==12==The signal is caused by a READ memory access.
==12==Hint: this fault was caused by a dereference of a high value address (see register values below).  Dissassemble the provided pc to learn which register was used.
    #0 0x7f29cfdfc03b in bootstrap_page_program sw/device/silicon_creator/rom/bootstrap.c:176
    #1 0x7f29cfdfc99e in bootstrap_handle_program sw/device/silicon_creator/rom/bootstrap.c:316
    #2 0x7f29cfdfce84 in bootstrap sw/device/silicon_creator/rom/bootstrap.c:386
    #3 0x561998b6049b in TestBody sw/device/silicon_creator/rom/bootstrap_unittest.cc:703
    #4 0x7f29cfbd6feb in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/googletest/googletest/src/gtest.cc:2607
    #5 0x7f29cfbca77d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/googletest/googletest/src/gtest.cc:2643
    #6 0x7f29cfb88ccb in testing::Test::Run() external/googletest/googletest/src/gtest.cc:2682
    #7 0x7f29cfb8a287 in testing::TestInfo::Run() external/googletest/googletest/src/gtest.cc:2861
    #8 0x7f29cfb8b3de in testing::TestSuite::Run() external/googletest/googletest/src/gtest.cc:3015
    #9 0x7f29cfbb1370 in testing::internal::UnitTestImpl::RunAllTests() external/googletest/googletest/src/gtest.cc:5855
    #10 0x7f29cfbda0a9 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/googletest/googletest/src/gtest.cc:2607
    #11 0x7f29cfbcd124 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/googletest/googletest/src/gtest.cc:2643
    #12 0x7f29cfbadec7 in testing::UnitTest::Run() external/googletest/googletest/src/gtest.cc:5438
    #13 0x7f29cfd37c75 in RUN_ALL_TESTS() external/googletest/googletest/include/gtest/gtest.h:2490
    #14 0x7f29cfd37b52 in main external/googletest/googlemock/src/gmock_main.cc:70
    #15 0x7f29cf229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #16 0x7f29cf229e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #17 0x561998b47f14 in _start (/home/dan_zerorisc_com/.cache/bazel/_bazel_dan/e8b9a1cec6d6126ab719828a176ad974/execroot/lowrisc_opentitan/bazel-out/k8-fastbuild/bin/sw/device/silicon_creator/rom/bootstrap_unittest+0x62f14)
@dmcardle dmcardle added the Type:Bug Bugs label Jul 6, 2023
@dmcardle dmcardle self-assigned this Jul 6, 2023
@dmcardle
Copy link
Contributor Author

dmcardle commented Jul 6, 2023

FYI @alphan

@dmcardle
Copy link
Contributor Author

dmcardle commented Jul 6, 2023

Per our chat offline, this is not a real concern. The SPI device driver will never generate values larger than 256, and the lowest value that can trigger this behavior is 257.

Before closing this issue, I'll add a comment to clarify the range of spi_device_cmd_t::payload_byte_count. I'll also update my fuzzer to stay within that range when it generates spi_device_cmd_t values.

dmcardle added a commit to dmcardle/opentitan that referenced this issue Jul 6, 2023
Fixes lowRISC#19132

Signed-off-by: Dan McArdle <dmcardle@opentitan.org>
@dmcardle dmcardle removed the Type:Bug Bugs label Jul 6, 2023
@alphan alphan closed this as completed in 6b04226 Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant