leveldown-security / SVD-Loader-Ghidra

GNU General Public License v3.0
450 stars 91 forks source link

failed to generate peripherals for nrf52.svd #1

Open fanoush opened 4 years ago

fanoush commented 4 years ago

Hi, this is great stuff. However I tried it with https://github.com/posborne/cmsis-svd/blob/master/data/Nordic/nrf52.svd and it only creates memory areas but nothing in Peripherals namespace. The output is:

SVD-Loader.py> Running...
Loading SVD file...
    Done!
Generating memory regions...
    Done!
Generating peripherals...
    FICR
        Failed to generate peripheral FICR
    UICR
        Failed to generate peripheral UICR
    BPROT
        Failed to generate peripheral BPROT
    POWER
        Failed to generate peripheral POWER
    CLOCK
        Failed to generate peripheral CLOCK
    RADIO
        Failed to generate peripheral RADIO
    UARTE0
        Failed to generate peripheral UARTE0
    SPIM0
        Failed to generate peripheral SPIM0
    SPIS0
        Failed to generate peripheral SPIS0
    TWIM0
        Failed to generate peripheral TWIM0
    TWIS0
        Failed to generate peripheral TWIS0
    SPI0
        Failed to generate peripheral SPI0
    TWI0
        Failed to generate peripheral TWI0
    SPIM1
        Failed to generate peripheral SPIM1
    SPIS1
        Failed to generate peripheral SPIS1
    TWIM1
        Failed to generate peripheral TWIM1
    TWIS1
        Failed to generate peripheral TWIS1
    SPI1
        Failed to generate peripheral SPI1
    NFCT
        Failed to generate peripheral NFCT
    GPIOTE
        Failed to generate peripheral GPIOTE
    SAADC
        Failed to generate peripheral SAADC
    TIMER0
        Failed to generate peripheral TIMER0
    TIMER1
        Failed to generate peripheral TIMER1
    TIMER2
        Failed to generate peripheral TIMER2
    RTC0
        Failed to generate peripheral RTC0
    TEMP
        Failed to generate peripheral TEMP
    RNG
        Failed to generate peripheral RNG
    ECB
        Failed to generate peripheral ECB
    CCM
        Failed to generate peripheral CCM
    AAR
        Failed to generate peripheral AAR
    WDT
        Failed to generate peripheral WDT
    RTC1
        Failed to generate peripheral RTC1
    QDEC
        Failed to generate peripheral QDEC
    COMP
        Failed to generate peripheral COMP
    LPCOMP
        Failed to generate peripheral LPCOMP
    SWI0
        Failed to generate peripheral SWI0
    EGU0
        Failed to generate peripheral EGU0
    SWI1
        Failed to generate peripheral SWI1
    EGU1
        Failed to generate peripheral EGU1
    SWI2
        Failed to generate peripheral SWI2
    EGU2
        Failed to generate peripheral EGU2
    SWI3
        Failed to generate peripheral SWI3
    EGU3
        Failed to generate peripheral EGU3
    SWI4
        Failed to generate peripheral SWI4
    EGU4
        Failed to generate peripheral EGU4
    SWI5
        Failed to generate peripheral SWI5
    EGU5
        Failed to generate peripheral EGU5
    TIMER3
        Failed to generate peripheral TIMER3
    TIMER4
        Failed to generate peripheral TIMER4
    PWM0
        Failed to generate peripheral PWM0
    PDM
        Failed to generate peripheral PDM
    NVMC
        Failed to generate peripheral NVMC
    PPI
        Failed to generate peripheral PPI
    MWU
        Failed to generate peripheral MWU
    PWM1
        Failed to generate peripheral PWM1
    PWM2
        Failed to generate peripheral PWM2
    SPIM2
        Failed to generate peripheral SPIM2
    SPIS2
        Failed to generate peripheral SPIS2
    SPI2
        Failed to generate peripheral SPI2
    RTC2
        Failed to generate peripheral RTC2
    I2S
        Failed to generate peripheral I2S
    P0
        Failed to generate peripheral P0
    TWI1
        Failed to generate peripheral TWI1
    UART0
        Failed to generate peripheral UART0
SVD-Loader.py> Finished!
fanoush commented 4 years ago

I have rethrown the exception in the code to see it and it fails with

Traceback (most recent call last):
  File "/home/fanoush/SVD-Loader-Ghidra/SVD-Loader.py", line 135, in <module>
    length = calculate_peripheral_size(peripheral)
  File "/home/fanoush/SVD-Loader-Ghidra/SVD-Loader.py", line 55, in calculate_peripheral_size
    size = max(size, register.address_offset + register._size/8)
TypeError: unsupported operand type(s) for /: 'NoneType' and 'int'

so I guess register._size is not there

fanoush commented 4 years ago

OK, register._size is optional, default is in peripheral.size so I have fixed it like this

 diff --git a/SVD-Loader.py b/SVD-Loader.py
index a55b7a5..689c68d 100644
--- a/SVD-Loader.py
+++ b/SVD-Loader.py
@@ -52,7 +52,8 @@ def reduce_memory_regions(regions):
 def calculate_peripheral_size(peripheral):
        size = 0
        for register in peripheral.registers:
-               size = max(size, register.address_offset + register._size/8)
+               rs = ( peripheral._size if (register._size is None) else register._size ) / 8
+               size = max(size, register.address_offset + rs)
        return size

@@ -142,14 +143,14 @@ for peripheral in peripherals:

                for register in peripheral.registers:
                        r_type = UnsignedIntegerDataType()
-                       rs = register._size / 8
+                       rs = ( peripheral._size if (register._size is None) else register._size ) / 8
                        if rs == 1:
                                r_type = ByteDataType()
                        elif rs == 2:
                                r_type = UnsignedShortDataType()
                        elif rs == 8:
                                r_type = UnsignedLongLongDataType()
-                       peripheral_struct.replaceAtOffset(register.address_offset, r_type, register._size/8, register.name, register.description)
+                       peripheral_struct.replaceAtOffset(register.address_offset, r_type, rs, register.name, register.description)

                addr = space.getAddress(peripheral_start)

However then I didn't get very far. Some peripherals still fail since they are overlapping which is intentional as they really share same space (how to solve this?) but even for those not overlapping it prints no error but the namespace does not look so rich as on video and analyzed code still looks almost same as before.

nezza commented 4 years ago

Heya, thanks for the feedback! I'll try to look at this in the next days and see what I can figure out!

nezza commented 4 years ago

Yea so this is an issue - for those overlaps we might just create names instead of structures..

Calvin69 commented 4 years ago

Hello, sorry for being late to the party. May I know if there is any solutions to this issue? Because I got the same exact error @fanoush had.

fanoush commented 4 years ago

@Calvin69 did you try my patch above to at least get rid of exception because of missing register._size value (which seems to be optional but the code expected it to be always present)?

Another isue is overlapping of address spaces. Just to explain how it is with nrf52 - there are 3 reasons for using same address space:

  1. older deprecated and newer (DMA based) version of same HW e.g. UART vs UARTE (E=EasyDMA)
  2. older deprected vs several variants of same iterface SPI vs SPIM (master with DMA) vs SPIS (slave with DMA)
  3. same HW block used for two mutually exclusive interfaces SPI and TWI=I2C

Type is selected by writing specific value to ENABLE 0x500 register. The list what is shared is here so e.g. 0x40003000 can be used for old style SPI0 or newer DMA based variants SPIS0 SPIM0 or older TWI0 or newer DMA based TWIM0 TWIS0. Selection is done by writing values 1,2,7 (SPI variants) or 5,6,9 (TWI variants) to 0x40003500.

Typically this selection for specific decompiled code is static so in theory it could be automagically detected from code writing specific value to ENABLE register however the easiest would be to simply set a preference at decompile/loading time which one is preferred and ignore the others.

nezza commented 4 years ago

Heya,

Yea so I have a basic solution working for overlapping address spaces, however I didn't get to fully test it yet unfortunately. In my basic solution I just generate a pspec for the processor, which then will not have structures but at least labels... However your solution with having the user select the right way sounds interesting! Maybe I'll create an SVD-Loader option to just name the memory addresses without creating structs, that would allow overlapping without issues, at the expense of worse names...

Cheers and thanks for investigating,

Thomas

Calvin69 commented 4 years ago

Hello guys,

I just realized that the errors on my console is different that @fanoush . Here's mine:

  File "C:\Users\Admin\ghidra_scripts\SVD-Loader-Ghidra-master\SVD-Loader.py", line 61, in <module>
    parser = SVDParser.for_xml_file(str(svd_file))
  File "C:\Users\Admin\ghidra_scripts\SVD-Loader-Ghidra-master\cmsis_svd\parser.py", line 72, in for_xml_file
    return cls(ET.parse(path), remove_reserved)
  File "C:\Users\Admin\Desktop\ghidra_9.1.2_PUBLIC\Ghidra\Features\Python\data\jython-2.7.1\Lib\xml\etree\ElementTree.py", line 1184, in parse
    tree.parse(source, parser)
  File "C:\Users\Admin\Desktop\ghidra_9.1.2_PUBLIC\Ghidra\Features\Python\data\jython-2.7.1\Lib\xml\etree\ElementTree.py", line 657, in parse
    self._root = parser.close()
  File "C:\Users\Admin\Desktop\ghidra_9.1.2_PUBLIC\Ghidra\Features\Python\data\jython-2.7.1\Lib\xml\etree\ElementTree.py", line 1667, in close
    self._raiseerror(v)
  File "C:\Users\Admin\Desktop\ghidra_9.1.2_PUBLIC\Ghidra\Features\Python\data\jython-2.7.1\Lib\xml\etree\ElementTree.py", line 1519, in _raiseerror
    raise err
  File "<string>", line None
xml.etree.ElementTree.ParseError: Attribute name "data-pjax-transient" associated with an element type "meta" must be followed by the ' = ' character.
SVD-Loader.py> Finished!

It doesn't seems to be an overlapping issue, isn't it? I have never modified any SVD Loader files. I had only modified a Ghidra file which is the ia.sinc which is a solution to this issue answered by nsadeveloper789. Could that be causing the issue?

@nezza are you GhidraNinja? If so, I'd like to say thanks for uploading videos on YT. Really helped me learn a lot.

UPDATE: Looking at the first line of error, I uploaded a .xml file and now I get the same error. Will try fanoush's method and will update again.

UPDATE 2: Just tried fanoush's patch and still get the same thing. No change.

SourceDiver42 commented 4 years ago

Hello, I have a similar problem when generating the peripherals but this time it is on the nrf51 platform. As this is an issue which is shared between the nrf51 and nrf52 processor, may I suggest changing the title of the issue to integrate nrf51 as well? Other than that, the overlap seems to be the only issue on my side.

EDIT: I double checked for the nrf51.svd file and indeed, some are requesting the same region. The alternatePeripheral tag references other failing peripherals. In my case for instance the peripheral CLOCK has the alternatePeripheral POWER, which has its own definition somewhere else.

nezza commented 4 years ago

Awesome, I'll try to review on the weekend!

Avamander commented 3 years ago

Yeah, I just encountered this as well with the nrf51.svd file.

bananabr commented 7 months ago

I do have similar problems with rp2040.svd.