Check that the frame pointer is within the current stack.
ClosedPublic
Actions

Authored by jhb on Nov 24 2020, 9:49 PM.

Details

Reviewers

manu
andrew
markj
gnn

Commits

rS368455: Check that the frame pointer is within the current stack.

Summary

This same check is used on other architectures. Previously this would
permit a stack frame to unwind into any arbitrary kernel address
(including unmapped addresses).

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 35197
Build 32143: arc lint + arc unit

Event Timeline

jhb created this revision.Nov 24 2020, 9:49 PM

Herald added a reviewer: manu. · View Herald TranscriptNov 24 2020, 9:49 PM

Herald added subscribers: emaste, andrew. · View Herald Transcript

jhb requested review of this revision.Nov 24 2020, 9:49 PM

Harbormaster completed remote builds in B35015: Diff 79958.Nov 24 2020, 9:49 PM

RISC-V has similar changes in D27357

jhb added reviewers: andrew, markj.Nov 24 2020, 9:56 PM

andrew added inline comments.Nov 26 2020, 2:22 PM

sys/cddl/dev/dtrace/aarch64/dtrace_isa.c
91–93	This was changed in rS333570 as `unwind_frame` could be probed by the FBT provider. It might be enough to exclude it from being probed in `fbt_provide_module_function`

markj added inline comments.Nov 26 2020, 3:07 PM

sys/cddl/dev/dtrace/aarch64/dtrace_isa.c
91–93	I think that would be fine. Alternately, perhaps unwind_frame() could move to stack.h?

jhb added inline comments.Dec 1 2020, 6:27 PM

sys/cddl/dev/dtrace/aarch64/dtrace_isa.c
91–93	Hmm, risc-v would need the same approach as I have made it also use unwind_frame. Do you have a preference Mark on how to fix it?

markj added inline comments.Dec 1 2020, 6:36 PM

sys/cddl/dev/dtrace/aarch64/dtrace_isa.c
91–93	I think I prefer to explicitly exclude unwind_frame() from FBT by modifying fbt_provide_module_function() for the relevant arches. All of the solutions (keep the code as-is, make unwind_frame() inlinable, exclude unwind_frame() from FBT) are kind of fragile, but modifying FBT at least makes it easy to identify these problematic functions.