Auth handler and restorer#8
Conversation
Previously restorer was only stripped, while handler was left untouched. Without authenticating kernel would receive a signed address and use it, as is, to call the signal handler, which would result in an invalid memory access.
jchlanda
force-pushed
the
jakub/handler_restorer_auth
branch
from
ece31aa to
5ef373a
Compare
Yeap, https://github.com/rust-lang/rust/blob/main/tests/ui/abi/stack-probes.rs#L1 - the original failing test that uncovered this for us - is passing with this patch. |
|
@kovdan01 I wonder if we have to account for |
We do, but our current musl implementation does not support it anyway (there are other similar places). So trying to account for it here would be pretending that we support that everywhere, while this is simply not true. I'll file an issue on this. @jchlanda Could you please add a has_feature check against fn type discr, and if it's present, just fall to a preprocessor error? |
See #9 |
@jchlanda Thanks! Just double-checking: the remaining tests are also passing, right? :) I mean, when experimenting with this yesterday and when switching from strip to auth, I had many other new failures. These are probably guarded now with your checks against |
Yeap, that would be my guess, trying to auth those two will obviously fail. All the rust tests are passing with this patch.
Aye, done. |
atrosinenko
left a comment
atrosinenko
left a comment
There was a problem hiding this comment.
Looks like this patch unintentionally "rendered" TABs as 8 spaces (accidentally spotted this as my Github interface preference is set to 4 spaces), though I'm not sure we care about this here.
@atrosinenko apologies, that’s from my clang-format. I can't see a formatting config in the repo; how do you usually handle code style here? |
I use this repo in a read-only manner most of the time :) Maybe @kovdan01 has something to suggest, but I guess we don't care for code style here as much as in the LLVM repo (where code style was semi-automatically enforced for a long time and thus it is much easier for the new patches to adhere to the rules). I'm not sure, maybe there is something similar in Musl upstream, but I'm not sure we are contributing there actively. To my understanding, this fork of Musl is kind of proof-of-concept for testing, @kovdan01 please correct me if I'm wrong. |
I've done something similar-looking to what we already have in the files. Also tabs, instead of spaces. |
|
@jchlanda Oh, I forgot to say that you can ask me to merge this PR on your behalf if no more changes are planned and no reviews are waited for :) |
That would be great, thank you. |
|
FYI In https://github.com/access-softek/pauth-toolchain-build-scripts the particular hashes of |
3 participants
Previously restorer was only stripped, while handler was left untouched. Without authenticating kernel would receive a signed address and use it, as is, to call the signal handler, which would result in an invalid memory access.