Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
941 stars 213 forks source link

Handle PAC related to instructions and access to x18 register on arm64 #6148

Open bb33bb opened 1 week ago

bb33bb commented 1 week ago

Bug Description: in Pseudo C code, when we are calculating the number of arguments, we treat the following reg of x18 as an argument: Here is the code

ffffffc0082effb0    void audit_core_dumps(int64_t* arg1, int64_t* arg2 @ x18)

ffffffc0082effb0  5f2403d5   bti     c
ffffffc0082effb4  5e8600f8   str     x30, [x18], #0x8
ffffffc0082effb8  3f2303d5   paciasp 
ffffffc0082effbc  ff4301d1   sub     sp, sp, #0x50
ffffffc0082effc0  fd7b02a9   stp     x29, x30, [sp, #0x20] {__saved_x29} {var_28}
ffffffc0082effc4  f65703a9   stp     x22, x21, [sp, #0x30] {__saved_x22} {__saved_x21}

I think it is not a special case in the specific function, I found it in many functions. But to be honest, the real definition of this function is

void audit_core_dumps(long signr)
{
    struct audit_buffer *ab;

    if (!audit_enabled)
        return;

    if (signr == SIGQUIT)   /* don't care for those */
        return;

    ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_ANOM_ABEND);
    if (unlikely(!ab))
        return;
    audit_log_task(ab);
    audit_log_format(ab, " sig=%ld res=1", signr);
    audit_log_end(ab);
}

And also, because this assemble instruction code locates before
paciasp as this is a Pac instruction, I don't know whether I am right. as my personal sense, Pac is added by the compiler. So, register of x18 is not designed by normal programmer, So, it is not the arguments of the original function. As a comparation of Ida, we get the follow code

void __fastcall audit_core_dumps(long signr)
{
  _QWORD *v1; // x18
  __int64 v2; // x30
  task_struct_26 *StatusReg; // x21
  audit_buffer *v5; // x0
  audit_buffer *v6; // x20
  size_t v7; // x0
  mm_struct *mm; // x0
  file_2 *mm_exe_file; // x0
  file *v10; // x21
  char buf[8]; // [xsp+8h] [xbp-18h] BYREF
  __int64 v12; // [xsp+18h] [xbp-8h]

  *v1 = v2;
  v12 = *(_QWORD *)(_ReadStatusReg(ARM64_SYSREG(3, 0, 4, 1, 0)) + 1504);
  if ( signr != 3 )
  {
    if ( audit_enabled )

a little urgly but maybe the true result.

If we need the binary plz call me.

bb33bb commented 6 days ago

i think i should upload these two files, because its really a little time confusing to reproduce . please wait a second, i am uploading

xusheng6 commented 6 days ago

i think i should upload these two files, because its really a little time confusing to reproduce .

Well I get what is happening, and I am probably going to close it as intended. Still, I will first discuss it with the team

Our philosophy is to produce the output that is closest to the behavior of the code. In this sense, the source code is only a reference and our decompiling captures the behavior of the assembly instructions better than that

On the other hand, it does not hurt anything to have x18 as the second argument. If you want, you can change the function type to get rid of it

bb33bb commented 6 days ago

OK~ the files are Here https://1drv.ms/f/c/01e018a652fc6a6e/Ej4Ra1Am3aZOgnvoAAcTKC8BRXfZ7cHC7o804U3X6CoNQQ

xusheng6 commented 6 days ago

Sorry that I did not notice x18 is related to PAC. I have updated the issue title accordingly

galenbwill commented 6 days ago

Vector 35 employees can search private slack for unknown chicken hammer bull