MIPS64: relocation issue

[mumbel]

trafficstars

Describe the bug MIPS64 relocation handler discards the old symbol value resulting in symbols having the EA as their value. This binary is built to load at 0x0, so basically all of my .rel.dyn relocations are NULL pointers.

For example readelf shows:

000000667788  000000001203 R_MIPS_REL32     
                    Type2: R_MIPS_64        
                    Type3: R_MIPS_NONE   

with file data of: 000000667788: 00 00 00 00 XX YY ZZ WW and ghidra loads: 000000667788: 00 00 00 00 00 00 00 00

R_MIPS_REL32 handling has saveValue true and symbolIndex 0x0, resulting in symbolValue 0x0 Fall through in R_MIPS_32 extractAddend() false

R_MIPS_64 handling has saveValue false, addend 0x0, and symbolValue 0x0 resulting in overwriting the previous value with 0x0

Environment (please complete the following information):

• OS: 20.04
• Java Version: 17.0.4
• Ghidra Version: 10.2-DEV b88cf85d5cef4e766e6093223b6320ee5ae7a113
• Ghidra Origin: eclipse

[ghidra1]

Could you please provide a binary which exhibits this behavior

[ghidra1]

It is possible that the section symbol is not picking up the address of the corresponding section block. Will need to investigate with a sample.

[mumbel]

@ghidra1 I'll see if I can find a shareable binary or if I can generate an ELF.

I did work around with (still playing with the image, so not sure how good/bad this was):

for reloc in currentProgram.getRelocationTable().getRelocations():
    if reloc.getType() == 0x1203 and 0 == reloc.getValues()[0]:
        setBytes(reloc.getAddress(), reloc.getBytes())

[ghidra1]

This binary is built to load at 0x0

Have you tried just changing the base import address to 0 instead of excepting the forced default (e.b., 0x10000) ? That is the purpose of the relocations. If the binary is not truely relocatable you need to force it to use the correct base address at import time.

[mumbel]

Sorry that wasn't clear, I did word that weird. I did change base address to 0. So with that and reverting the relocations it looks good. Sorry I have been unable to find a shareable sample so far, plan to look more this weekend

[ghidra1]

We do sometime struggle with pre-linked relocateable binaries - could this be one of those? The relocations are present but do not need to be applied. In those cases disabling relocation processing from import options may be the appropriate solution.

[mumbel]

It's seems to be just the symbolIndex==0 maybe or maybe just the .rel.dyn section, I'll double check to see if I notice any patterns. There were a lot of relocations untouched by the above script

[ghidra1]

I believe the intent of this relocation for symbolIndex==0 is:

R_MIPS_REL32
   value = imageBaseAdjustment + addend
   ***Assuming this is a REL type relocation, value should equal the original 
       bytes (32-bits) from the relocation address
   This value should be saved for use below
R_MIPS_64
   addend = value from above
   newValue = 0 + addend
   store value as 64-bit long

***Could the issue be caused by the R_MIPS_REL32 using the upper 32-bits instead of the lower 32-bits of the affected 64-bit location? It is using a 32-bit oldValue read from the relocation address. Since this code is shared for both 32-bit and 64-bit MIPS that oldValue read may be flawed for 64-bit MIPS and should have been a 64-bit read.

[mumbel]

Could the issue be caused by the R_MIPS_REL32 using the upper 32-bits instead of the lower 32-bits of the affected 64-bit location? It is using a 32-bit oldValue read from the relocation address. Since this code is shared for both 32-bit and 64-bit MIPS that oldValue read may be flawed for 64-bit MIPS and should have been a 64-bit read.

ohhhhh, yeah... that is looking like the issue. I added a int tmpOldValue = memory.getInt(relocationAddress.add(4)); and got my original address

[ghidra1]

The danger of a 32/64-bit consolidated handler :(

I will document and put in for repair based on this. It could easily impact other data relocations which use an extracted addend.

[mumbel]

Thanks!

[ghidra1]

@mumbel Can you please test this modified MIPS_ElfRelocationHandler before I commit? Thanks! MIPS_ElfRelocationHandler.java.txt

[mumbel]

@ghidra1 Yes, looks good as far as I can tell for the R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE relocations. Thanks again (and thanks for adding/fixups for the xhash PR)

[ryanmkurtz]

Closed by 8c1fd57b72c84d99e8dca998ff0d1c22161bc4c4