radareorg / radare2

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

View graph (VV) gives different/broken results when loading with ihex:// #8512

Closed Modiug closed 5 years ago

Modiug commented 7 years ago

$ r2 -v radare2 1.7.0-git 16099 @ linux-x86-64 git.1.6.0-834-g52ebe07f4 commit: 52ebe07f49aed7ec0fed726de81d45c26a8f3589 build: 2017-09-13__22:06:32

To reproduce: unzip: exmple.zip

r2 -AA -a 8051 ihex://Vend_Ax.hex [0x00000000]> VV

and compare to

r2 -AA -a 8051 Vend_Ax.bin [0x00000000]> VV

Vend_Ax.bin was produced from Vend_Ax.hex by: objcopy -I ihex -O binary Vend_Ax.hex Vend_Ax.bin

radare commented 7 years ago

Are the contents the same?

On 14 Sep 2017, at 19:00, Guido Muesch notifications@github.com wrote:

To reproduce: unzip: exmple.zip

r2 -AA -a 8051 ihex://Vend_Ax.hex [0x00000000]> VV

and compare to

r2 -AA -a 8051 Vend_Ax.bin [0x00000000]> VV

Vend_Ax.bin was produced from Vend_Ax.hex by: objcopy -I ihex -O binary Vend_Ax.hex Vend_Ax.bin

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

condret commented 7 years ago

show me your radare2rc

Modiug commented 7 years ago

@radare: If by contents you mean Vend_Ax.hex and Vend_Ax.bin: Yes, the .bin was created from .hex with objcopy as stated. If you mean the attachement from the other ihex:// issue: The Vend_Ax.hex is the same, but I added the Vend_Ax.bin.

@condret: I do not have a radare2rc. At least not that I know off. No ~/.radare2rc, no radare2rc in .config/radare2, no .radare2rc in the current directory.

condret commented 7 years ago

is e io.ff set to true?

Modiug commented 7 years ago

Yes for both versions (.hex and .bin), if that would matter. [0x00000000]> e io.ff true

radare commented 7 years ago
screen shot 2017-09-15 at 12 07 38
radare commented 7 years ago

tahts not the same

radare commented 7 years ago

and siol removed the ability to specify a different and siol broke io.0xff.... so this cant be workarounded.. @condret pls we need tests for 0xff too

Modiug commented 7 years ago

The .bin and .hex files seem to be different, because objcopy fills gaps with 0 by default. It seems radare2 fills gaps with 0xff when reading ihex://. Try: objcopy -I ihex -o binary --gap-fill 255 Vend_Ax.hex Vend_Ax.bin

Still different? Why would it affect the graph visualization?

radare commented 7 years ago

because ff is an invalid instruction and 00 is not

radare commented 7 years ago

also, paste a screenshot to document the issue

Modiug commented 7 years ago

I now generated an Vend_Ax.bin where the gaps are filled with 0xff. The loaded binaries look the same. See: bin: startup_bin ihex: startup_ihex

However, when I then type VV, I get: bin: vv_bin ihex: vv_ihex

radare commented 7 years ago

check the output of the om command, probably the analysis is different because of mapping without the exec bit

radare commented 7 years ago

cc @condret @MaskRay

also we want to want to have io.0xff back

Modiug commented 7 years ago

Same output for ihex and bin: [0x00000000]> om 1 fd: 3 +0x00000000 0x00000000 - 0x00000ead -r-x

radare commented 7 years ago

you can use io.0xff=0 to make r2 behave like objcopy by default

On 18 Sep 2017, at 23:19, Guido Muesch notifications@github.com wrote:

Same output for ihex and bin: [0x00000000]> om 1 fd: 3 +0x00000000 0x00000000 - 0x00000ead -r-x

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/radare/radare2/issues/8512#issuecomment-330359563, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-liTBRSyuMPkKrL_WGn9I8_SD7J9eks5sjt5igaJpZM4PX4-l.

Maijin commented 7 years ago

Maybe when opening with ihex:// this should be set by default ?

radare commented 7 years ago

I think the default should be 0xff to be 0 on interleaved regions and ff for unallocated. But this must be done in the ihex plugin itself

On 19 Sep 2017, at 18:31, Maijin notifications@github.com wrote:

Maybe when opening with ihex:// this should be set by default ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

brainstorm commented 6 years ago

I can confirm the issue, same symptoms and the analysis for my AVR ESC firmwares seems to differ a bit from the bin (left hand side) to the hex (right hand side):

skarmavbild 2018-04-22 kl 16 58 19
radare commented 6 years ago

Whats the point in blaming the graph when its a problem with the io?

On 22 Apr 2018, at 08:54, Roman Valls Guimerà notifications@github.com wrote:

I can confirm this bug with my AVR firmwares, I've observed same insights and issues. The problems seem to start early by i.e not detecting/enforcing arch bits correctly:

motor1 romanvg$ r2 -aavr -b16 flash_blc.bin -- Hang in there, Baby! [0x00000026]> motor1 romanvg$ r2 -aavr -b16 ihex://eeprom_blc.hex Cannot set bits 64 to 'avr' Cannot set bits 64 to 'avr' -- Welcome back, lazy human! [0x00000000]> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

brainstorm commented 6 years ago

My bad, sorry I don't have a full understanding on how io/siol/ihex load the bits to internal data structures :-S

radare commented 6 years ago

Any noticeable difference in the io data comparing plain posix io with ihex://? Or it is a glitch? Just open both files side by side and scroll until you see a difference

On 23 Apr 2018, at 04:21, Roman Valls Guimerà notifications@github.com wrote:

My bad, sorry I don't have a full understanding on how io/siol/ihex load the bits to internal data structures :-S

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

brainstorm commented 6 years ago

I did precisely that, scrolling for a while in the beginning end and random sections, side by side, and nope, no differences eyeballing the hexes. Here are some screenshots:

skarmavbild 2018-04-24 kl 13 44 26 skarmavbild 2018-04-24 kl 13 44 02 skarmavbild 2018-04-24 kl 13 43 37

The corresponding hex and bin were uploaded to the AVR ESIL Telegram channel a couple of days ago.

radare commented 6 years ago

The problem is that the analysis don't have a way to detect if the read operation fails because it reads 000000 or ffffffs, even if the hexdump shows 0000s. So if you objdump with 0xff instead of 0x00 you will get same behaviour in both files.

this is a bug in the way io.0xff eval var is managed.

radare commented 5 years ago

Probably the same issue as https://github.com/radare/radare2-regressions/pull/1574

radare commented 5 years ago

May be good to test again with io.unalloc

brainstorm commented 5 years ago

Similar errors on my side with e io.unalloc=true in ~/.radare2rc:

$ r2 ~/dev/esc_ardrone/avr_russians/motor1/flash_blc.hex
 -- git pull now
[0x00000000]> aaaa
[Invalid instruction of 4054 bytes at 0x2and entry0 (aa)
[x] Analyze all flags starting with sym. and entry0 (aa)
[x] Analyze function calls (aac)
[x] Analyze len bytes of instructions for references (aar)
[x] Constructing a function name for fcn.* and sym.func.* functions (aan)
[x] Enable constraint types analysis for variables
$ r2 ~/dev/esc_ardrone/avr_russians/motor1/flash_blc.bin
WARNING: Plugin avr should implement load_buffer method instead of load_bytes.
 -- vm is like a small cow in ascii
[0x00000026]> aaaa
[x] Analyze all flags starting with sym. and entry0 (aa)
[x] Finding xrefs in noncode section with anal.in = 'io.maps'
[x] Analyze value pointers (aav)
[x] Value from 0x00000000 to 0x00001f42 (aav)
[x] 0x00000000-0x00001f42 in 0x0-0x1f42 (aav)
[x] Emulate code to find computed references (aae)
[SPM: I dont know what to do with SPMCSR 00.
SPM: I dont know what to do with SPMCSR 00.
SPM: I dont know what to do with SPMCSR 00.
[x] Analyze function calls (aac)
[SPM: I dont know what to do with SPMCSR 00.ferences (aar)
SPM: I dont know what to do with SPMCSR 00.
[x] Analyze len bytes of instructions for references (aar)
[x] Constructing a function name for fcn.* and sym.func.* functions (aan)
SPM: I dont know what to do with SPMCSR 00.
[x] Enable constraint types analysis for variables
radare commented 5 years ago

in the debugger i also observe some differences when doing the analysis. may be related

brainstorm commented 5 years ago

Tested locally right now both ihex:// and .bin locally, finally the analysis is consistent (equal) on both formats, well done folks! :)

Thanks @Maijin for the heads up on testing this ;)

radare commented 5 years ago

awesome <3 took 3 years to catch this bug

On 10 Sep 2019, at 12:46, Roman Valls Guimera notifications@github.com wrote:

Tested locally right now both ihex:// and .bin locally, finally the analysis is consistent (equal) on both formats, well done folks! :)

Thanks @Maijin https://github.com/Maijin for the heads up on testing this ;)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/radare/radare2/issues/8512?email_source=notifications&email_token=AAG75FSJ5GSVAEC4LR5KODTQI53IZA5CNFSM4D27R6S2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6KVGWI#issuecomment-529879897, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG75FQDAR5A2BFZP2STVLLQI53IZANCNFSM4D27R6SQ.