radareorg / radare2

UNIX-like reverse engineering framework and command-line toolset
https://www.radare.org/
GNU Lesser General Public License v3.0
20.28k stars 2.97k forks source link

Wrong rjmp interpretation in AVR on ATmega644/640 #14078

Open TrueXakeP opened 5 years ago

TrueXakeP commented 5 years ago

Work environment

Questions Answers
OS/arch/bits (mandatory) Windows 10 1809 17763.437 x86 64bit
File format of the file you reverse (mandatory) BIN ROM
Architecture/bits of the file (mandatory) AVR8, ATmega644/640
r2 -v full output, not truncated (mandatory) radare2 3.5.0-git 21238 @ windows-x86-64 git.3.5.0-git commit: 1c279c6081bc1877fabd533a8314eb527e32c66c build: Wed 04/10/2019__10:52:39.32

Expected behavior

Arrow from first rjmp at 0xb8da must point to sleep before it at 0xb8d8 (PC-1). Like rjmp at 0xc in test-short.bin from this screenshot. Arrow from second rjmp at 0xb928 must point to nop at 0xb92e (PC+2) following the next call test-short.bin displays expected behavior. Like rjmp at 0x5a in test-short.bin from this screenshot. correct

Graph view from address of first rjmp must show short loop. graph-correct

Actual behavior

Arrows in print of pd 50 at [0xb8d0] shows that rjmp on 0xb8da and on 0xb928 are jumping far back, while their calculated jump addresses are correct. e asm.cpu=ATmega640 nothing changes. bug

Graph view is broken too. graph-bug

Steps to reproduce the behavior

test.bin displays the problem. test-short.bin displays the expected

1) bug in arrows r2 -a avr test.bin or test-short.bin > e asm.cpu=ATmega640 > 0xb8d0 // for test.bin (.org 0x5C6B 2 - 6) or stay at 0x0 for test-short.bin [0xb8d0]> pd 50 see difference in arrows from rjmp's at first pair of screenshots between test.bin and test-sort.bin Note that the br* restart instructions are binary the same in booth cases and their arrow are shown correct in booth cases.*

2) bug in graph go to address of first rjmp 0xb8da for test.bin or 0xc for test-short.bin aaa VV see difference in graph at second pair of screenshots

test.zip test-short.zip

test-short.bin compiled with .org 0x04 of main_loop instead of .org 0x5C6B

source code of test.bin which displays the problem is taken from real firmware dump:

.org 0x00 ; __RESET
                jmp main
; ...
.org 0x5C6B ; << important
main_loop:
                nop
loop:
                sleep
                rjmp loop ; << here
main:
                ldi     r16, 0x8f ; reset the stack ptr, no return
                out     SPL, r16        
                ldi     r16, 0x10
                out     SPH, r16        
                ldi     r28, 0
                ldi     r29, 0x11
                cli
                set
                clh
                ses
                clv
                sen
                clz
                sec
                brie    restart
                brtc    restart
                brhs    restart
                brge    restart
                brvs    restart
                brpl    restart
                breq    restart
                brcc    restart
                sei
                clt
                seh
                cls
                sev
                cln
                sez
                clc
                brid    restart
                brts    restart
                brhc    restart
                brlt    restart
                brvc    restart
                brmi    restart
                brne    restart
                brcs    restart
                rjmp    continue ; << and here
restart:
                call    main
continue:
                nop ; ... many code of main
                call main_loop
                jmp main_loop

Additional Logs, screenshots, source-code, configuration dump, ...

Cutter is also show wrong graph for call main.

From the datasheet:

The ATmega644 Program Counter (PC) is 15/16 bits wide, thus addressing the 32/64K program memory locations.

Sorry, I'm not familiar with r2 yet. And sorry for my English.

TrueXakeP commented 5 years ago

I'll test now.

TrueXakeP commented 5 years ago

Just tested with "radare2 3.5.1 1 @ windows-x86-64 git.3.5.1 commit: 4ec482ba2d4f554beeafb3aa47758e970bcab937 build: Wed 05/15/2019__ 7:58:44.85" The arrows of rjmp in disassembly print are correct for ATmega1280, ATmega1281, ATmega2560, ATmega2561 and ATxmega128a4u as the asm.cpu. But not others and not ATmega640. Functions then must be redefined to fix the graph rendering.

But now I see that the aaa turns nop at 0xb92e into the .dword 0x940e0000: изображение изображение

In real firmware aaa turns another nop follows ret in data: изображение

revyakin commented 4 years ago

ATMega644 has a 16-bit wide program counter, but ATmega640 has only 15 bit PC. So your jump addresses have been truncated down to 15 bit. (eg. 0xb8d8 -> 0x38d8).

You could choose a different asm.cpu, perhaps ATMega1280. Or you can define a new MCU type in libr/anal/p/anal_avr.c