Skip to content

Conversation

@droe
Copy link
Contributor

@droe droe commented Nov 17, 2025

Add UC_CTL_PAUTH_SIGN, UC_CTL_PAUTH_STRIP and UC_CTL_PAUTH_AUTH as an architecture-independent interface to pointer authentication operations without having to execute instructions on the virtual CPU. This is useful in many scenarios, for instance when rebasing signed pointers as part of loading code that runs with PAuth.

The C interfaces are covered by a unit test. The Python interfaces have been tested in a real world project.

Minor changes included:

  • Fix Python __ctl_wr to work for n input 1 output parameter
  • Rename confusingly named CAS unit test (suffix _pac) to avoid confusion with PAuth unit test
  • Add myself to CREDITS.TXT since this is I think my fifth PR

droe added 6 commits November 17, 2025 23:24
Add UC_CTL_PAUTH_SIGN, UC_CTL_PAUTH_STRIP and UC_CTL_PAUTH_AUTH as an
architecture-independent interface to pointer authentication operations
without having to execute instructions on the virtual CPU.  This is
useful in many scenarios, for instance when rebasing signed pointers as
part of loading code that runs with PAuth.

The C interfaces are covered by a unit test.  The Python interfaces have
been tested in a real world project.
@droe droe marked this pull request as draft November 21, 2025 12:02
@droe
Copy link
Contributor Author

droe commented Nov 21, 2025

Converted to draft for now, some of the Windows and alpine test failures do look related to PAuth. Not sure how my changes would be the root cause of these failures just yet, could also be dormant bugs uncovered by the tests I added? Will investigate. If someone actually using the Windows or alpine builds of Unicorn wants to give me a hand, please reach out.

@droe droe force-pushed the droe/arm64-pauth-sign branch 2 times, most recently from bb34ca0 to fb3adb1 Compare November 23, 2025 23:15
@droe
Copy link
Contributor Author

droe commented Nov 23, 2025

So for alpine-x86 and windows-x64 MINGW32, the segfault happens because in uc_ctl(), for some reason, signed_ptr ends up as NULL instead of the value that was passed in. Similarly, on windows-x86 MSVC 32bit, signed_ptr ends up as a non-NULL pointer, but a different pointer than was passed in, causing the test to fail. Some calling convention issue in the uc_ctl() interface?

The odd one out is windows-x64 MINGW64, where the signed_ptr pointer is correct, that one may or may not have a different root cause.

@wtdcode
Copy link
Member

wtdcode commented Nov 24, 2025

So for alpine-x86 and windows-x64 MINGW32, the segfault happens because in uc_ctl(), for some reason, signed_ptr ends up as NULL instead of the value that was passed in. Similarly, on windows-x86 MSVC 32bit, signed_ptr ends up as a non-NULL pointer, but a different pointer than was passed in, causing the test to fail. Some calling convention issue in the uc_ctl() interface?

The odd one out is windows-x64 MINGW64, where the signed_ptr pointer is correct, that one may or may not have a different root cause.

This sounds like a memory corruption happening elsewhere. Unfortunately there seems no memory sanitizer on Windows but maybe you could try ASAN/MSAN on Linux?

droe added 2 commits November 24, 2025 21:59
Fixes issue where literal arguments would be written to variable
argument memory as an int, but subsequently read from va_list as a
uint64_t, which on some platforms might be a different size and lead to
corruption of later arguments, breaking PAuth helper functionality or
causing segmentation faults.
@droe droe force-pushed the droe/arm64-pauth-sign branch 2 times, most recently from acabb19 to 209f87d Compare November 24, 2025 21:32
@droe
Copy link
Contributor Author

droe commented Nov 24, 2025

My hunch was correct, uc_ctl() calling conventions were to blame, of sorts:

The implementation pulls uint64_t's from the variable argument list:

uc_err uc_ctl(uc_engine *uc, uc_control_type control, ...)
{
    // ...
    va_list args;
    // ...
    case UC_CTL_PAUTH_SIGN: {

        UC_INIT(uc);

        if (rw == UC_CTL_IO_READ_WRITE) {
            uint64_t ptr = va_arg(args, uint64_t);
            int key = va_arg(args, int);
            uint64_t diversifier = va_arg(args, uint64_t);
            uint64_t *signed_ptr = va_arg(args, uint64_t *);
            if (uc->pauth_sign != NULL) {
                err = uc->pauth_sign(uc, ptr, key, diversifier, signed_ptr);
            } else {
                // ...

We wrap the vararg uc_ctl() call in the following macro:

#define uc_ctl_pauth_sign(uc, ptr, key, diversifier, signed_ptr)               \
    uc_ctl(uc, UC_CTL_READ_WRITE(UC_CTL_PAUTH_SIGN, 4), (ptr), (key), (diversifier), (signed_ptr))

That we then call as if it was a function:

    OK(uc_ctl_pauth_sign(uc, some_unsigned_pointer, UC_ARM64_PAUTH_KEY_IA, 0, &signed_pointer));

However, note the literal 0 there. It's going to be pushed to the variable argument list as an int, which may or may not be the same size as a uint64_t, and independent from its size, may or may not use 64 bits in the variable argument list, depending on the platform. On macOS and mainstream Linux, all was well, but on some other platforms, this resulted in the variable argument reading to go off the rails, corrupting the signed_pointer pointer that is being read after the type-size-mismatched diversifier.

The simple fix would be to require callers to use 0ULL instead of 0. But since the uc_ctl_* convenience wrappers look like functions and users would be likely to make the same mistake as I made, I went for the more robust fix of type-casting all parameters in the macro, e.g.:

#define uc_ctl_pauth_sign(uc, ptr, key, diversifier, signed_ptr)               \
    uc_ctl(uc, UC_CTL_READ_WRITE(UC_CTL_PAUTH_SIGN, 4), (uint64_t)(ptr), (int)(key), (uint64_t)(diversifier), (uint64_t *)(signed_ptr))

Pushed a fix, removed the debug code. This should be ready to go in once the CI completes.

@droe droe marked this pull request as ready for review November 24, 2025 21:34
@droe
Copy link
Contributor Author

droe commented Nov 26, 2025

Failed CI checks are unrelated to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants