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.

Sign up for GitHub

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

Jump to bottom

context gives wrong jump target for jalr ra, ra, -0x16 in RISC-V #2173

Closed
Ordoviz opened this issue May 18, 2024 · 2 comments · Fixed by #2177
Closed

context gives wrong jump target for jalr ra, ra, -0x16 in RISC-V #2173

Ordoviz opened this issue May 18, 2024 · 2 comments · Fixed by #2177
Labels
bug

Comments

@Ordoviz
Copy link
Contributor

Ordoviz commented May 18, 2024

The jump target shown as annotation in the disassembly window is wrong:

pwndbg> si
pwndbg> set emulate on
pwndbg> context
 ► 0x1008e <_start+4>    jalr   ra, ra, -0x16               <main+8>
pwndbg> si
pwndbg> context
 ► 0x10074 <main>    addi   a7, zero, 0x40

Interestingly, nearpc shows the correct jump target:

 ► 0x1008e <_start+4>    jalr   ra, ra, -0x16               <main>

A workaround is set emulate off.

test binary

Download test binary. I created it from:

# riscv32-unknown-linux-gnu-as -march=rv32imac2p0 -o riscv-emu-bug.o riscv-emu-bug.s
# riscv32-unknown-linux-gnu-ld --no-relax -o riscv-emu-bug riscv-emu-bug.o

.section .rodata

greeting: .asciz "Hello world\n"
.equ greetlen, . - greeting

.section .text
main:
        li  a7, 64         # write
        li  a0, 1
        la  a1, greeting
        li  a2, greetlen
        ecall
        ret

.global _start
_start:
        call main
        li  a7, 93         # exit
        li  a0, 0
        ecall

version information

pwndbg version: a1ddb3c (my fork)

Platform: Linux-6.9.1-arch1-1-x86_64-with-glibc2.39
OS: Arch Linux
OS ABI: #1 SMP PREEMPT_DYNAMIC Fri, 17 May 2024 16:56:38 +0000
Architecture: x86_64
Endian: little
Charset: utf-8
Width: 119
Height: 58
Gdb:      14.2
Python:   3.12.3 (main, Apr 23 2024, 09:16:07) [GCC 13.2.1 20240417]
Pwndbg:   2024.02.14 build: a1ddb3c0
Capstone: 5.0.1280
Unicorn:  2.0.1
This GDB was configured as follows:
   configure --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
	    --with-auto-load-dir=$debugdir:$datadir/auto-load
	    --with-auto-load-safe-path=$debugdir:$datadir/auto-load
	    --with-expat
	    --with-gdb-datadir=/build/share/gdb (relocatable)
	    --with-jit-reader-dir=/build/lib/gdb (relocatable)
	    --without-libunwind-ia64
	    --with-lzma
	    --without-babeltrace
	    --without-intel-pt
	    --with-xxhash
	    --with-python=/usr
	    --with-python-libdir=/usr/lib
	    --with-debuginfod
	    --with-curses
	    --with-guile
	    --without-amd-dbgapi
	    --enable-source-highlight
	    --enable-threading
	    --enable-tui
	    --with-system-readline
	    --with-separate-debug-dir=/build/lib/debug (relocatable)
	    --with-system-gdbinit=/etc/gdb/gdbinit
@Ordoviz Ordoviz added the bug label May 18, 2024
@OBarronCS
Copy link
Contributor

I found the source of the bug in the code that handles resolving RISCV targets:

pwndbg/pwndbg/disasm/riscv.py

Lines 97 to 103 in e812ef0

if instruction.id in [RISCV_INS_JALR, RISCV_INS_C_JALR]:
target = (
self.parse_register(instruction, instruction.op_find(CS_OP_REG, 1), emu)
+ instruction.op_find(CS_OP_IMM, 1).imm
) & ptrmask
# Clear the lowest bit without knowing the register width
return target ^ (target & 1)

The call to self.parse_register(instruction, instruction.op_find(CS_OP_REG, 1), emu) will resolve the value of the ra register, but at this point in the code logic we have already stepped the emulator, so it gets the value that jalr writes to ra. Then we calculate the jump target using this incorrect value.

Previously in the control flow we have already resolved the values of all register before/after emulation, so we can just access them immediately. Made a PR to fix this (and replaces two other such parse_register calls with direct access to the already-resolved register value).

@OBarronCS OBarronCS mentioned this issue May 20, 2024
Merged
@disconnect3d
Copy link
Member

Thanks for reporting this. It should be fixed in #2177. Thanks @OBarronCS :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

riscv jalr jump resolution bug fix OBarronCS/pwndbg
3 participants
@disconnect3d @Ordoviz @OBarronCS