fruityloops1 / nim-prodinfo-blank-fix

Patches for nim that fix a 17.0.0+ crash after blanking prodinfo
27 stars 3 forks source link

Clarification to patch #1

Closed borntohonk closed 9 months ago

borntohonk commented 9 months ago

Hi, i saw your issue / feature request in the atmosphere repository, and i'm considering adding this to my fork, which embeds other patches such as the nifm, but could you clarify why you pass a mov 0x #0x0 instead of NOP's to the function responsible? (and why not a mov x0, xzr -> ret ( E0 03 1F AA C0 03 5F D6) )

You're kind stopping 3 function callbacks rather than just the one responsible

https://github.com/borntohonk/Atmosphere/commit/435e0a47b42d9f86a1fafa4ae511159544c76a07

if you're interested in seeing an example of how to implement it into atmosphere and enabled by default, then the parts from my commit about nifm is how to do that.

NOP

borntohonk commented 9 months ago

just as comment from this code structure since your patch of returning 0 in this if conditonal block

7100133598 4a 00 00 94 bl FUN_71001336c0 undefined FUN_71001336c0()

this function returns returns a response equal to zero, causing the chain to go on.

  71001335a8 2e a2 fc 97     bl         FUN_710005be60                                   undefined FUN_710005be60()

this function is then called, and if zero then the next chain is set, which is then assigned value 0x89a74

to me it seems the root cause of the chain and issue is that function FUN_71001336c0 called at 0x133698 is the cause of the process chain being set off. (and that obviously returns 0 to set off the chain)

borntohonk commented 9 months ago

I feel like just altering 0X1336B0 to E0 03 1F AA (mov, x0, xzr) from 44 00 00 94 (BL FUN_71001336c0) and making it just be 0 should accomplish the same result of returning 0 without inteferring with the function loop like you are doing. movx0xzr

fruityloops1 commented 9 months ago

I think you have the wrong version of nim's main nso open. What's the build id of the nso file? It should be C14BC0AD5027F6B6B49A5A6B2D52D5E8306EE1D2 for 17.0.0/17.0.1. 7100133760 is the start of a whole function, which I've singled out to be solely for the abort. image FUN_7100002260 is the one that then calls nn::diag::detail::AbortImpl.

borntohonk commented 9 months ago

Seems i was 0x100 off, my pseudocode for function doesn't generate like yours, not sure if my ghidra is misconfigured or not, but whatever.

so the reason you cut off that function and return 0 that early is to return 0 for the comparator at 71001334a0 )80 0f 00 35 cbnz w0,LAB_7100133690) , which loads 0x1336C0 which is related to what I was looking at above. the value you are setting to zero is used in FUN_71001336c0 at 71001334b8

every function that calls the function at FUN_71001336c0 are expecting zero to be returned succeeds, else it returns non-zero and is what is returning you a non-zero and causing your error to spawn.

ret0

flow

why not just patch: 71001334a8 c2 15 00 10 adr x2,LAB_7100133760

to: 71001334a8 e2 03 1f aa mov x2,xzr true0

borntohonk commented 9 months ago

C14BC0AD5027F6B6B49A5A6B2D52D5E8306EE1D2 1334A8 0004E2031FAA

(the reason im claryfing this is because i write a heuristic updater to batch-process all patches needed (if needed) for future firmware updates, and if i'm adding this patch to my fork, i'd rather just have it be proper before adding it.)

https://github.com/borntohonk/Atmosphere/blob/master/stratosphere/loader/source/ldr_patcher.cpp#L112 https://github.com/borntohonk/Atmosphere/blob/master/stratosphere/loader/source/ldr_embedded_nim_patches.inc https://github.com/borntohonk/Atmosphere/blob/master/stratosphere/loader/source/ldr_patcher.cpp#L169-L175

https://github.com/borntohonk/Atmosphere/releases/tag/17.0.0-1.6.2-9610100de

boots fine, runs fine, have ams blanker on, networks fine.

i don't have a cursed prodinfo though, only people with incognito/incognito_rcm applied are the ones that crash, but my system is stable with this patch embedded into my fork. I can't really validate if it cures the crash symptom for people who have incognito applied previously though, but it's now in my fork.

(i used to maintain incognito_rcm, and am kind of the reason this issue exists, so incorporating a fix for issue that has arised in 17.0.0+ is neat)


import re
import os

#(https://github.com/borntohonk/Switch-Ghidra-Guides/blob/master/scripts/nxo64.py)
import nxo64

nimcompressed = f'main'

def get_nim_build_id():
    with open(nimcompressed, 'rb') as f:
        f.seek(0x40)
        return f.read(0x14).hex().upper()

nimbuildid = get_nim_build_id()

with open('main', 'rb') as compressed_nim_nso:
    nxo64.write_file(f'uncompressed_nim.nso0', nxo64.decompress_nso(compressed_nim_nso))
    with open('uncompressed_nim.nso0', 'rb') as decompressed_nim_nso:
        read_data = decompressed_nim_nso.read()
        result = re.search(rb'\x80\x0f\x00\x35\x1f\x20\x03\xd5', read_data)
        if not result:
            print("regex broken")
        else:
            text_file = open('./%s.ips' % nimbuildid, 'wb')
            patch = '%06X%s%s' % (result.end(), '0004', 'E2031FAA')
            offset = '%06X' % (result.end())
            text_file.write(bytes.fromhex(str(f'5041544348{patch}454F46')))
            text_file.close()  
    decompressed_nim_nso.close()
    compressed_nim_nso.close()
fruityloops1 commented 9 months ago

You can certainly do that, but I don't see a difference other than it making the patch 4 bytes smaller. I added it here. Also, you don't have to write scripts to generate IPS files, IPSwitch generates them just fine.

borntohonk commented 9 months ago

You can certainly do that, but I don't see a difference other than it making the patch 4 bytes smaller. I added it here. Also, you don't have to write scripts to generate IPS files, IPSwitch generates them just fine.

I have them output formatted strings for my fork too, the one there is just a generalized one without the strings.