From 4fd337fb24f306265e45d353acd108291b322cbc Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Fri, 9 Oct 2026 11:13:32 +0200 Subject: [PATCH] Fix the netboot launcher: relocate the copier, reserve the staging region The launcher committed in b438960 would have hung the console, and reviewing it against Dennis's suggestion to test with Swiss's boot.dol is what surfaced it -- Swiss is known-good, so a hang could only have been the launcher, but I would not have been able to tell that from "the transfer is broken". Two distinct defects, both from the same oversight -- a DOL's sections load from 0x80003100 upward, which is exactly where this program is running: 1. The copier was an ordinary C function in .text, so it overwrote its OWN instructions partway through the copy. 2. The image was staged in a memalign'd buffer, which the destination sections can overwrite while it is still being read from. Fixes: - netboot_stub.S: the copy/zero/cache-flush/jump sequence in position independent assembly, copied into reserved memory and executed THERE. It has no globals, no literal pool, no TOC references and no calls, because any of those would read from the address it was LINKED at rather than the one it is RUNNING at. It also does the dcbf/icbi pass the C version was doing through DCFlushRange -- without it the CPU executes stale icache lines at the entry point. - netboot_reserve(): carves the top of MEM1 and hands it to SYS_SetArena1Hi() so neither libogc nor malloc allocates there. Called FIRST in main(), before the framebuffer. - netboot_launch(): verifies every text/data section AND bss against the reserved region and refuses (-3) if any would land on it, instead of corrupting the staged image or the copier mid-flight. Also hands the new image a stack inside the reserved tail. The app now tells you on screen to send Swiss's boot.dol first, and the README explains why: the launcher and the transfer can each be broken independently and a hang looks identical, so the first send should be an image whose success is unmistakable. Co-Authored-By: Claude Opus 5 --- gc_bba_test/README.md | 32 ++++++++- gc_bba_test/source/main.c | 43 +++++++----- gc_bba_test/source/netboot.c | 108 ++++++++++++++++++++++++------ gc_bba_test/source/netboot.h | 11 +++ gc_bba_test/source/netboot_stub.S | 94 ++++++++++++++++++++++++++ 5 files changed, 245 insertions(+), 43 deletions(-) create mode 100644 gc_bba_test/source/netboot_stub.S diff --git a/gc_bba_test/README.md b/gc_bba_test/README.md index cb15067..bc98f30 100644 --- a/gc_bba_test/README.md +++ b/gc_bba_test/README.md @@ -107,6 +107,32 @@ Failure behaviour, so a bad update never costs a power cycle: - `netboot_launch()` refuses an image whose entry point or first text section is not in MEM1, rather than jumping into a mis-parsed header. -The one unavoidable risk is the jump itself: once sections are copied and -interrupts are off, a bad image hangs the console and needs a power cycle. The -header check above is what stands between a truncated transfer and that. +### Send Swiss's `boot.dol` first + +The launcher and the transfer are separate things that can each be broken, and +a hang looks the same either way. Send a **known-good** image the first time — +Swiss's `boot.dol` is ideal, because it is unmistakable when it boots. If +Swiss comes up, both halves work and you can then trust an untested build. + +### Why the copier lives in assembly at the top of MEM1 + +A DOL's sections load from 0x80003100 upward — exactly where this program's own +`.text` sits. A copier compiled as an ordinary C function would **overwrite its +own instructions partway through the copy** and hang. So: + +- `netboot_reserve()` carves the top of MEM1 and hands it to + `SYS_SetArena1Hi()`, so neither libogc nor `malloc` will touch it. The image + is staged there, never in heap memory. +- `netboot_stub.S` is copied into that region and executed *there*, clear of + everything the image writes. It must stay strictly position independent: no + globals, no literal pool, no TOC references, no calls — otherwise it reads + from the address it was *linked* at instead of the one it is *running* at. +- `netboot_launch()` checks every section (and BSS) against the reserved + region and **refuses** if any would land on it, rather than corrupting the + image or the copier mid-flight. + +The remaining risk is the jump itself: once sections are copied and interrupts +are off, a bad image hangs the console and needs a power cycle. The header and +overlap checks are what stand between a truncated transfer and that — they +cannot validate a *plausible* but broken image, which is why the first send +should be Swiss. diff --git a/gc_bba_test/source/main.c b/gc_bba_test/source/main.c index d708d13..4552812 100644 --- a/gc_bba_test/source/main.c +++ b/gc_bba_test/source/main.c @@ -21,6 +21,8 @@ #include "netboot.h" static void *xfb = NULL; +static void *s_stage = NULL; /* netboot staging region (top of MEM1) */ +static u32 s_stage_max = 0; static GXRModeObj *rmode = NULL; #define C_RESET "\x1b[37;0m" @@ -125,25 +127,23 @@ static void nb_status(const char *msg) static void do_netboot(char *status, size_t nstatus) { - void *buf; - int len; + int len, rc; printf("\n" C_HEAD " Self-update over the BBA" C_RESET "\n\n"); - printf(" On the host: sudo ./scripts/send-dol.py \n"); - printf(" B on the next screen is NOT available -- this blocks until the\n" - " transfer finishes or times out (30 s idle, 5 s mid-transfer).\n\n"); + printf(" On the host: sudo ./scripts/send-dol.py \n\n"); + printf(C_DIM " Send a KNOWN-GOOD image the first time -- Swiss's boot.dol\n" + " is ideal: it is unmistakable when it boots, so it tells you the\n" + " transfer AND the launcher both work before you trust either\n" + " with an untested build.\n" C_RESET "\n"); + printf(" Blocks until done or it times out (30 s idle, 5 s mid-xfer).\n\n"); - /* 8 MB ceiling: far above any .dol, and it keeps a corrupt length field - * from trying to allocate the whole heap. */ - buf = memalign(32, 8u << 20); - if (!buf) { - snprintf(status, nstatus, "net-update: out of memory"); + if (!s_stage) { + snprintf(status, nstatus, "net-update: staging region unavailable"); return; } - len = netboot_receive(buf, 8u << 20, nb_status); + len = netboot_receive(s_stage, s_stage_max, nb_status); if (len <= 0) { - free(buf); snprintf(status, nstatus, "net-update failed (%d)", len); return; } @@ -154,12 +154,13 @@ static void do_netboot(char *status, size_t nstatus) VIDEO_WaitVSync(); VIDEO_WaitVSync(); - if (netboot_launch(buf) < 0) { - free(buf); - snprintf(status, nstatus, "not a valid DOL - not launching"); - return; - } - /* netboot_launch() does not return on success. */ + rc = netboot_launch(s_stage); + /* Only reached on refusal. -2 = header implausible (truncated or not a + * DOL); -3 = a section would land on the reserved staging region, which + * would corrupt the image or the copier mid-flight. */ + snprintf(status, nstatus, + rc == -3 ? "refused: image overlaps staging region" + : "refused: not a plausible DOL"); } int main(void) @@ -168,6 +169,12 @@ int main(void) int sel = 0, ssel = 0, cur = 0; char status[80] = ""; + /* Reserve the netboot staging region FIRST, before libogc allocates the + * framebuffer or anything else out of the arena. A staged image must + * not live in ordinary heap memory: DOL sections load from 0x80003100 + * upward and would overwrite it partway through the copy. */ + s_stage = netboot_reserve(&s_stage_max); + VIDEO_Init(); PAD_Init(); rmode = VIDEO_GetPreferredMode(NULL); diff --git a/gc_bba_test/source/netboot.c b/gc_bba_test/source/netboot.c index 782746b..1e435cc 100644 --- a/gc_bba_test/source/netboot.c +++ b/gc_bba_test/source/netboot.c @@ -188,42 +188,106 @@ typedef struct { u32 pad[7]; } dolhdr_t; +/* Reserved region at the top of MEM1: + * + * [ staged image .............. ][ stub ][ descriptor ][ new stack ] + * ^ s_stage ^ s_stub + * + * Nothing here may overlap any DOL section, and netboot_launch() checks that + * before it jumps rather than trusting it. */ +#define NB_STAGE_MAX (6u << 20) /* generous: Swiss is ~2-3 MB */ +#define NB_TAIL (64u << 10) /* stub + descriptor + stack */ + +extern void nb_stub(void *desc); +extern void nb_stub_end(void); + +static u8 *s_stage; +static u8 *s_tail; + +void *netboot_reserve(u32 *out_max) +{ + u32 hi = (u32)SYS_GetArena1Hi(); + u32 base = (hi - (NB_STAGE_MAX + NB_TAIL)) & ~31u; + + if (base <= (u32)SYS_GetArena1Lo()) return NULL; + s_stage = (u8 *)base; + s_tail = (u8 *)(base + NB_STAGE_MAX); + /* Tell libogc this is not its memory any more. */ + SYS_SetArena1Hi(s_stage); + if (out_max) *out_max = NB_STAGE_MAX; + return s_stage; +} + +static int overlaps(u32 a, u32 alen, u32 b, u32 blen) +{ + if (!alen || !blen) return 0; + return a < b + blen && b < a + alen; +} + int netboot_launch(const void *dol) { const dolhdr_t *h = (const dolhdr_t *)dol; const u8 *base = (const u8 *)dol; - void (*entry)(void); - int i; + u32 reserved = (u32)s_stage; + u32 reserved_len = NB_STAGE_MAX + NB_TAIL; + u32 stub_len = (u32)((const u8 *)nb_stub_end - (const u8 *)nb_stub); + u32 *desc; + void (*stub)(void *); + int i, nsec = 0; - /* Sanity: the entry point and at least one text section must live in - * MEM1. Jumping to a mis-parsed header hangs the console hard enough - * to need a power cycle, so refuse rather than guess. */ + if (!s_stage) return -1; + + /* Refuse a mis-parsed or truncated header rather than jumping into it: + * a bad jump hangs the console hard enough to need a power cycle. */ if ((h->entry >> 24) != 0x80 || h->text_len[0] == 0 || (h->text_addr[0] >> 24) != 0x80) - return -1; - - /* No interrupts from here on: we are about to overwrite the memory this - * program is running from, and with interrupts off the scheduler cannot - * preempt us into code that no longer exists. */ - IRQ_Disable(); + return -2; + /* Build the copy table in the reserved tail, where the image cannot + * reach it. While doing so, verify no section lands on the reserved + * region -- that would clobber the staged image, the stub we are about + * to run from, or this very table. */ + desc = (u32 *)(s_tail + 0x1000); for (i = 0; i < 7; i++) { if (!h->text_len[i]) continue; - memmove((void *)h->text_addr[i], base + h->text_off[i], h->text_len[i]); - DCFlushRange((void *)h->text_addr[i], h->text_len[i]); - ICInvalidateRange((void *)h->text_addr[i], h->text_len[i]); + if (overlaps(h->text_addr[i], h->text_len[i], reserved, reserved_len)) + return -3; + desc[5 + nsec * 3 + 0] = (u32)(base + h->text_off[i]); + desc[5 + nsec * 3 + 1] = h->text_addr[i]; + desc[5 + nsec * 3 + 2] = h->text_len[i]; + nsec++; } for (i = 0; i < 11; i++) { if (!h->data_len[i]) continue; - memmove((void *)h->data_addr[i], base + h->data_off[i], h->data_len[i]); - DCFlushRange((void *)h->data_addr[i], h->data_len[i]); - } - if (h->bss_len) { - memset((void *)h->bss_addr, 0, h->bss_len); - DCFlushRange((void *)h->bss_addr, h->bss_len); + if (overlaps(h->data_addr[i], h->data_len[i], reserved, reserved_len)) + return -3; + desc[5 + nsec * 3 + 0] = (u32)(base + h->data_off[i]); + desc[5 + nsec * 3 + 1] = h->data_addr[i]; + desc[5 + nsec * 3 + 2] = h->data_len[i]; + nsec++; } + if (h->bss_len && + overlaps(h->bss_addr, h->bss_len, reserved, reserved_len)) + return -3; - entry = (void (*)(void))h->entry; - entry(); + desc[0] = h->entry; + /* Hand the new image a stack inside the reserved tail, 64-byte aligned + * and clear of the descriptor. */ + desc[1] = ((reserved + reserved_len) - 64) & ~63u; + desc[2] = (u32)nsec; + desc[3] = h->bss_addr; + desc[4] = h->bss_len; + + /* Relocate the copier out of harm's way and make it executable there. */ + stub = (void (*)(void *))s_tail; + memcpy(s_tail, (const void *)nb_stub, stub_len); + DCFlushRange(s_tail, NB_TAIL); + ICInvalidateRange(s_tail, NB_TAIL); + + /* No interrupts from here on: we are about to overwrite the memory this + * program is running from, and the scheduler must not preempt us into + * code that no longer exists. */ + IRQ_Disable(); + stub(desc); return 0; /* not reached */ } diff --git a/gc_bba_test/source/netboot.h b/gc_bba_test/source/netboot.h index efca8ad..edc7d1f 100644 --- a/gc_bba_test/source/netboot.h +++ b/gc_bba_test/source/netboot.h @@ -29,6 +29,17 @@ #define NB_BLOCK_MAX 1024u /* payload bytes per frame */ +/* Reserve the staging region at the top of MEM1. MUST be called early in + * main(), before anything else allocates, and the pointer it returns is where + * netboot_receive() should stage the image. Returns NULL if MEM1 is too + * small (it never is on real hardware). + * + * The image CANNOT live in an ordinary malloc'd buffer: DOL sections load from + * 0x80003100 upward and would overwrite it partway through the copy. This + * carves the top of MEM1 and hands it to SYS_SetArena1Hi so libogc will not + * allocate there either. */ +void *netboot_reserve(u32 *out_max); + /* Receive a .dol into `dst`. Returns its length, or <0 on error. * `status` (optional) is called with a human-readable progress line. */ int netboot_receive(void *dst, u32 maxlen, diff --git a/gc_bba_test/source/netboot_stub.S b/gc_bba_test/source/netboot_stub.S new file mode 100644 index 0000000..a16397c --- /dev/null +++ b/gc_bba_test/source/netboot_stub.S @@ -0,0 +1,94 @@ +/* Relocatable DOL copier for netboot_launch(). + * + * WHY THIS EXISTS IN ASSEMBLY, IN ITS OWN FILE: + * a DOL's sections load over the memory this program is running from (the + * standard load address is 0x80003100, right where our own .text sits), so a + * copier compiled as an ordinary C function would overwrite its own + * instructions partway through the copy and hang the console. This routine is + * copied into a reserved region at the top of MEM1 and executed THERE, so the + * image cannot touch it. + * + * It must therefore be strictly position independent: no globals, no literal + * pool, no TOC references, no calls. Only PC-relative branches and + * register-relative loads. Anything else silently reads from the address it + * was LINKED at rather than the one it is RUNNING at. + * + * r3 = descriptor, in the same reserved region: + * 0x00 entry point + * 0x04 stack pointer to hand the new image + * 0x08 section count + * 0x0C bss address + * 0x10 bss length + * 0x14 section count * (src, dst, len) + */ + .text + .globl nb_stub + .globl nb_stub_end + .align 2 + +nb_stub: + /* ---- copy every section ---- */ + lwz 4, 8(3) /* nsec */ + addi 5, 3, 0x14 /* -> first (src,dst,len) */ +1: cmpwi 4, 0 + beq 3f + lwz 6, 0(5) /* src */ + lwz 7, 4(5) /* dst */ + lwz 8, 8(5) /* len */ + cmpwi 8, 0 + beq 2f + mtctr 8 +4: lbz 9, 0(6) + stb 9, 0(7) + addi 6, 6, 1 + addi 7, 7, 1 + bdnz 4b +2: addi 5, 5, 12 + addi 4, 4, -1 + b 1b + +3: /* ---- zero BSS ---- */ + lwz 7, 0xC(3) + lwz 8, 0x10(3) + cmpwi 8, 0 + beq 6f + li 9, 0 + mtctr 8 +5: stb 9, 0(7) + addi 7, 7, 1 + bdnz 5b + +6: /* ---- make the copies visible to the instruction fetcher ---- + * Flush each destination out of the data cache and invalidate the + * matching instruction-cache lines. Without this the CPU happily + * executes stale instructions from icache at the entry point. */ + lwz 4, 8(3) + addi 5, 3, 0x14 +7: cmpwi 4, 0 + beq 9f + lwz 7, 4(5) + lwz 8, 8(5) + cmpwi 8, 0 + beq 8f + addi 8, 8, 31 /* round up to whole 32-byte lines */ + srwi 8, 8, 5 + mtctr 8 +10: dcbf 0, 7 + icbi 0, 7 + addi 7, 7, 32 + bdnz 10b +8: addi 5, 5, 12 + addi 4, 4, -1 + b 7b + +9: sync + isync + lwz 1, 4(3) /* stack for the new image */ + lwz 9, 0(3) /* entry */ + mtlr 9 + li 3, 0 + li 4, 0 + li 5, 0 + blr + +nb_stub_end: