binaryninja-api icon indicating copy to clipboard operation
binaryninja-api copied to clipboard

Provide way to easily specify clobbered registers for Function type objects.

Open yrp604 opened this issue 2 years ago • 8 comments

Binary Ninja Version 3017

Describe the bug

Originally reported by @nshp here, just copy/pasting so it has its own report as I hit this on a different binary: https://github.com/Vector35/binaryninja-api/issues/1769

There are no Function objects for these extern functions, so I don't think it's possible to set function properties for them. I'd specifically like to be able to override clobbered_regs for fentry (the hook point for ftrace), because it's called from every function and it saves/restores (or doesn't touch) every register. This means every function starts like:

int64_t bnx2x_ptp_adjtime()

00000710  int64_t rsi_1
00000710  void* rdi_1
00000710  rsi_1, rdi_1 = __fentry__()

with all the function parameters being misattributed to fentry.

To Reproduce Steps to reproduce the behavior:

  1. Go to kernote_ioctl
  2. Try to define arguments
  3. See __fentry__ eat them all

Expected behavior I am able to usefully define arguments to functions that begin with calls to __fentry__. Extern's having real Function objects that allow me to control clobbered registers is the way that seems easiest to me, but the root issue is just dataflow analysis on functions that call externs this way.

Version and Platform (required):

  • Binary Ninja: Dev 2.4.3017
  • OS: Windows
  • Version 20h1

Additional context kernote.ko.txt

yrp604 avatar Sep 30 '21 00:09 yrp604

There is a workaround that is probably preferable here:

class NopCallingConvention(CallingConvention):
  pass

bv.arch.register_calling_convention(NopCallingConvention(bv.arch, 'nop'))

the set the type of fentry to void __convention("nop") __fentry(void). Because of #2661, you actually have to override the call type to get it to stick.

This turns:

00000050  int64_t kernote_ioctl()

00000050      int64_t rdx_2
00000050      int32_t rsi_2
00000050      rdx_2, rsi_2 = __fentry__()
00000068      int64_t rsi
00000068      int64_t rdi
00000068      rsi, rdi = _raw_spin_lock(0xb40)

into:

00000050  int64_t kernote_ioctl(int64_t arg1, int32_t arg2, int64_t arg3)

00000050      __fentry__()
00000068      int64_t rsi
00000068      int64_t rdi
00000068      rsi, rdi = _raw_spin_lock(0xb40)

yrp604 avatar Sep 30 '21 07:09 yrp604

So with the other bugs resolved, I've solved this problem via a plugin: https://github.com/yrp604/binja-fentry

The request in the first comment and title is wrong, the second comment is accurate -- the plugin just makes the calling convention application automatic. This plugin (imo) should be rolled into a default binja kernel module support, but Im not sure how you'd like to do bookkeeping for this issue.

yrp604 avatar Oct 04 '21 22:10 yrp604

I think the fundamental issue here is there is no easy way to specify clobbered registers on externals, not necessarily the lack of function objects. Thus if we provided an easy way to specify clobbered registers. Then you'd have everything you need correct?

plafosse avatar Oct 12 '21 13:10 plafosse

Yes, that’s correct.

yrp604 avatar Oct 12 '21 16:10 yrp604

+1 this would be a really useful feature

DanielWood avatar Jul 14 '22 10:07 DanielWood

Implemented as of ~1 year ago via the "edit function properties" dialog.

jonpalmisc avatar Jul 14 '22 13:07 jonpalmisc

Hey @jonpalmisc I'm on Binary Ninja 3.1.3469 and it looks like its not fixed for this particular case.

In ELF files the external function symbols in .extern can't be right clicked to edit their properties (the option isn't there). The decompiler still makes assumptions about which registers are clobbered by these externals and there isn't any way to change it from the UI.

This leads to the problem yrp604 mentioned where reversing Linux kernel modules with __fentry__ at the start of the function messes up decompilation because the arguments passed in registers are now thought to come from __fentry__.

DanielWood avatar Jul 14 '22 22:07 DanielWood

You are correct: external symbols with function type (e.g., those in .extern) do not have function objects associated with them. (IIUC, functions have to have code mapped in the binary view.)

This means that there is no function object on which to set the clobbered registers. ~~I don't know of any workaround for this.~~

Correction: the plugin linked in here is a workaround: https://github.com/Vector35/binaryninja-api/issues/2663#issuecomment-933902584

galenbwill avatar Jul 14 '22 22:07 galenbwill