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

Furi: Detect use-after-free #294

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

Conversation

Willy-JL
Copy link
Member

@Willy-JL Willy-JL commented Nov 9, 2024

What's new

  • mirror of OFW PR 3995
  • free() now marks memory with 0xDD
  • BusFault handler checks for 0xDDDDDDDD and 64K after it to determine "Possible use-after-free"
  • fixed atomicity in furi_semaphore_release():
    • if thread A is waiting for acquire() and thread B calls release()
    • then halfway through release() code, A would wake up and continue, before B finishes release()
    • this could cause use-after-free if A free()s semaphore after acquire(), because after this B would finish release() which dereferences instance->event_loop_link
    • for example, RpcCli had this use-after-free:
      • rpc_cli_command_start_session() is waiting for furi_semaphore_acquire()
      • rpc_cli_session_terminated_callback() will furi_semaphore_release()
      • this wakes up rpc_cli_command_start_session() which then does furi_semaphore_free()
      • later, furi_semaphore_release() inside of rpc_cli_session_terminated_callback() resumes, and dereferences the semaphore that rpc_cli_command_start_session() has already free'd
    • there might be more similar issues with event loop items being used after yielding, which would break atomicity and lead to possible use-after-free, but i have not looked for them or found other crashes like this yet

For the reviewer

  • I've uploaded the firmware with this patch to a device and verified its functionality
  • I've confirmed the bug to be fixed / feature to be stable

if thread A waiting for acquire(), thread B calls release()
halfway through release() code, A would
wake up and continue, before B finishes release()
this could cause use-after-free if A free()s semaphore after acquire
@Willy-JL Willy-JL self-assigned this Nov 9, 2024
@Willy-JL Willy-JL added the enhancement New enhancement or request label Nov 9, 2024
Copy link

github-actions bot commented Nov 9, 2024

Compiled f7 firmware for commit 6485d7f1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New enhancement or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant