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.

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

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

Open
bb33bb opened this issue Nov 17, 2024 · 5 comments
Open

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

bb33bb opened this issue Nov 17, 2024 · 5 comments
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Medium Issue should take < 1 month Impact: Low Issue is a papercut or has a good, supported workaround Type: Enhancement Issue is a small enhancement to existing functionality

Comments

@bb33bb
Copy link

bb33bb commented Nov 17, 2024

  • Binary Ninja Version: 4.2.6447-dev Personal (5ca6dc49)
  • OS: manjaro
  • OS Version:
  • cat /proc/version
    Linux version 6.10.13-3-MANJARO (builduser@fv-az1246-770) (gcc (GCC) 14.2.1 20240910, GNU ld (GNU Binutils) 2.43.0) use Caches folder on OS X for download cache to prevent backups #1 SMP PREEMPT_DYNAMIC Tue Oct 8 03:24:49 UTC 2024

  • CPU Architecture: x64

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.

@xusheng6 xusheng6 added the State: Awaiting Triage Issue is waiting for more in-depth triage from a developer label Nov 19, 2024
@bb33bb
Copy link
Author

bb33bb commented Nov 19, 2024

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
Copy link
Member

xusheng6 commented Nov 19, 2024

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

@xusheng6 xusheng6 changed the title calculate the number of arguments Extra funciton arguments Nov 19, 2024
@bb33bb
Copy link
Author

bb33bb commented Nov 19, 2024

@xusheng6 xusheng6 changed the title Extra funciton arguments Handle PAC related to instructions and access to x18 register on arm64 Nov 19, 2024
@xusheng6 xusheng6 added Type: Enhancement Issue is a small enhancement to existing functionality Component: Architecture Issue needs changes to an architecture plugin Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Impact: Low Issue is a papercut or has a good, supported workaround Effort: Medium Issue should take < 1 month and removed State: Awaiting Triage Issue is waiting for more in-depth triage from a developer labels Nov 19, 2024
@xusheng6
Copy link
Member

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

@galenbwill
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Medium Issue should take < 1 month Impact: Low Issue is a papercut or has a good, supported workaround Type: Enhancement Issue is a small enhancement to existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants