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

Avoid use-after-free when disabling local tracepoint #5862

Closed

Conversation

jeremyevans
Copy link
Contributor

Disabling a local tracepoint while handling a different tracepoint
can cause a use-after-free in exec_hooks_body in certain cases,
because exec_hooks_body accessed local_hooks freed earlier in
iseq_remove_local_tracepoint.

Fix this by changing local_hooks in rb_iseq_struct from
struct rb_hook_list_struct * to struct rb_hook_list_struct **.
This allows us to set *local_hooks to NULL after freeing
*local_hooks. Functions now check both local_hooks and
*local_hooks, and only run hooks if both are non-NULL.

Addresses use-after-free crash found while investigating #18730.

Disabling a local tracepoint while handling a different tracepoint
can cause a use-after-free in exec_hooks_body in certain cases,
because exec_hooks_body accessed local_hooks freed earlier in
iseq_remove_local_tracepoint.

Fix this by changing local_hooks in rb_iseq_struct from
struct rb_hook_list_struct * to struct rb_hook_list_struct **.
This allows us to set *local_hooks to NULL after freeing
*local_hooks.  Functions now check both local_hooks and
*local_hooks, and only run hooks if both are non-NULL.

Addresses use-after-free crash found while in investigating #18730.
@jeremyevans jeremyevans requested review from ko1 and XrXr April 28, 2022 22:36
XrXr added a commit to XrXr/ruby that referenced this pull request Apr 29, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global
hooks which led to use-after-free when the global hook frees the
local hook list, for example by disabling a local TracePoint.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.
XrXr added a commit to XrXr/ruby that referenced this pull request Apr 29, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global
hooks which led to use-after-free when the global hook frees the
local hook list, for example by disabling a local TracePoint.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.
@XrXr
Copy link
Member

XrXr commented Apr 29, 2022

It seemed weird to me that adding another allocation is necessary and I debugged this down to an ordering problem in vm_trace_hook(). It seems to at least pass the test case here. The Redmine issue might need more investigation, though. My attempt is at: #5865

XrXr added a commit to XrXr/ruby that referenced this pull request May 28, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.

[Bug #18730]
XrXr added a commit to XrXr/ruby that referenced this pull request May 28, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.

[Bug #18730]
XrXr added a commit to XrXr/ruby that referenced this pull request May 28, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.

[Bug #18730]
XrXr added a commit that referenced this pull request May 30, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in GH-5862.

[Bug #18730]
schneems pushed a commit to schneems/ruby that referenced this pull request Jul 26, 2022
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in rubyGH-5862.

[Bug #18730]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants