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.

Sign up for GitHub

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

Jump to bottom

drmemtrace: Incorrect kernel xfer marker for signals received at end of wrapped function #5784

Closed
abhinav92003 opened this issue Dec 10, 2022 · 1 comment · Fixed by #5788 or #5896
Closed

drmemtrace: Incorrect kernel xfer marker for signals received at end of wrapped function #5784

abhinav92003 opened this issue Dec 10, 2022 · 1 comment · Fixed by #5788 or #5896
Assignees
@abhinav92003 @lihasgupta
Labels
Bug-ToolFail Component-DrMemtrace

Comments

@abhinav92003
Copy link
Contributor

A proprietary app hit the "Signal handler return point incorrect" invariant error. This happens for signals received at the end of a function that was wrapped with drwrap.

view output before the error at 23871501:

 23871110 17417534: T751373 <marker: function #9> // wrapped function starts
 23871111 17417534: T751373 <marker: function return address 0xaaaae88c138c>
...
 23871133 17417549: T751373 ifetch       4 byte(s) @ 0x0000aaaae891ee04 d65f03c0   ret    %x30  // wrapped function ends
 23871134 17417549: T751373 <marker: kernel xfer from 0xaaaae6bf81a8 to handler> // signal arrives: wrong marker value. This should be 0xaaaae88c138c, but it's probably replace_retaddr_sentinel.
... <signal processing>
 23871496 17417807: T751373 <marker: syscall xfer from 0xaaaae6bffcf0> // signal returns
 23871499 17417807: T751373 <marker: function #9> // drwrap post-wrap callback
 23871500 17417807: T751373 <marker: function return value 0x3528260f6028>
 23871501 17417808: T751373 ifetch       4 byte(s) @ 0x0000aaaae88c138c // back to actual app return address

-no_record_replace_retaddr resolves this, but we should fix it for -record_replace_retaddr too.
abhinav92003 added a commit that referenced this issue Dec 14, 2022
@abhinav92003
i#5784: Fix false positives in invariant checker
2cf8e26 
Fixes two false positives in the drcachesim invariant checker code.
Skips the pre-signal instr check when returning from signals that
arrived before any other instr in the trace, or before any other
instr since the last signal kernel xfer.

Adds tests that fail without this fix.

Fixes: #5784
@abhinav92003 abhinav92003 mentioned this issue Dec 14, 2022
Merged
abhinav92003 added a commit that referenced this issue Dec 15, 2022
@abhinav92003
i#5784: Fix false positives in invariant checker (#5788)
614a3c9 
Fixes two false positives in the drmemtrace invariant checker code. Skips the pre-signal instr check when returning from signals that arrived before any other instr in the trace, or before any other instr since the last kernel xfer (nested signals without any intervening instr). In both these cases we don't have any pre-signal instr whose pc we can compare with the post-signal instr.

Note that we still do not handle the back-to-back signals case where the second signal arrives just after the first one returns, without any intervening instruction. Added a comment for that.

Adds tests that fail without this fix.

Fixes: #5784
@abhinav92003
Copy link
Contributor Author

The issue reported in the first comment is actually not resolved yet. #5788 fixes a different type of "Signal handler return point incorrect" error.

@abhinav92003 abhinav92003 reopened this Dec 16, 2022
abhinav92003 added a commit that referenced this issue Mar 7, 2023
@abhinav92003
i#5784: Mitigate drwrap transparency violation
db392c5 
Adds a new drwrap API, drwrap_replace_if_retaddr_sentinel, that allows
mitigation of a transparency violation under the DRWRAP_REPLACE_RETADDR
drwrap strategy where the return address on the stack is replaced with
the address of the internal replace_retaddr_sentinel() routine. This
API modified the passed-in value to the actual return address of the
wrapped function if the passed-in value is replace_retaddr_sentinel()
itself.

Fixes the value of the marker written by the kernel xfer event in
drmemtrace by using the new drwrap_replace_if_retaddr_sentinel() API
on the mcontext PC before writing it out to the trace. This caused
many invariant errors of type 'Signal handler return point incorrect'
in traces collected on proprietary apps.

Verified on a large proprietary app that this error due to drwrap is
fixed now, whereas there were a few hundred instances before.

Fixes: #5784
@abhinav92003 abhinav92003 mentioned this issue Mar 7, 2023
Merged
abhinav92003 added a commit that referenced this issue Mar 7, 2023
@abhinav92003
i#5784: Mitigate drwrap retaddr transparency violation (#5896)
04dfa77 
Adds a new drwrap API, drwrap_get_retaddr_if_sentinel(), that allows
mitigation of a transparency violation under the DRWRAP_REPLACE_RETADDR drwrap
strategy where the return address on the stack is replaced with the address of
the internal replace_retaddr_sentinel() routine. This API modifies the passed-in
value to the actual return address of the inner-most nested wrapped function if
the passed-in value is replace_retaddr_sentinel() itself.

Fixes the value of the marker written by the kernel xfer event in drmemtrace by
using the new drwrap_get_retaddr_if_sentinel() API on the mcontext PC before
writing it out to the trace. Before, this caused many invariant errors of type 'Signal
handler return point incorrect' in traces collected on proprietary apps.

Verified on a large proprietary app that this error due to drwrap is fixed now,
whereas there were a few hundred instances before.

Fixes: #5784
@abhinav92003 abhinav92003 self-assigned this Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abhinav92003 abhinav92003

@lihasgupta lihasgupta

Labels
Bug-ToolFail Component-DrMemtrace
Projects
None yet
Milestone
No milestone
2 participants
@abhinav92003 @lihasgupta