ps2link: Fix IOP exception-report packet overflow

.github/workflows/compilation.yml

@@ -12,6 +12,15 @@ on:
   workflow_dispatch: {}
 
 jobs:
+  exception-packet-test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Validate IOP exception packet layout and bounds (plain C)
+        run: |
+          cc -std=c99 -Wall -Wextra -Werror tests/exception_packet.c -o /tmp/ps2link-exception-packet-test
+          /tmp/ps2link-exception-packet-test
+
   build:
     runs-on: ubuntu-latest
     container: ghcr.io/ps2dev/ps2-packer:latest
@@ -60,7 +69,7 @@ jobs:
           path: ee/ps2link.elf
 
   release:
-    needs: [build]
+    needs: [build, exception-packet-test]
     runs-on: ubuntu-latest
     if: startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/master'
     steps:

iop/Makefile

@@ -36,3 +36,5 @@ clean:
 
 include $(PS2SDK)/Defs.make
 include $(PS2SDK)/samples/Makefile.iopglobal
+
+$(IOP_OBJS_DIR)excepHandler.o: exception_packet.h

iop/excepHandler.c

@@ -9,10 +9,9 @@
 #include "irx_imports.h"
 #include "hostlink.h"
 #include "cmdHandler.h"
+#include "exception_packet.h"
 
-#define BUFFER_SIZE sizeof(exception_frame_t) + 4 + 4 + 128
-
-u32 excep_buffer[BUFFER_SIZE / 4] __attribute__((aligned(16)));
+static pko_iop_exception_packet_t excep_buffer __attribute__((aligned(16)));
 
 // taken from smod by mrbrown, only use one function, didn't wanna include another irx.
 
@@ -70,37 +69,27 @@ static void excep_handler2(exception_frame_t *frame)
 {
     u32 r_epc; // relative epc
     char *module_name;
-    u32 len;
-    u32 *buffer = excep_buffer;
 
     module_name = ExceptionGetModuleName(frame->epc, &r_epc);
 
     // Exceptions can occur outside a loaded module's text range.
     if (module_name == NULL) {
         module_name = "unknown";
         r_epc = frame->epc;
-        len = sizeof("unknown") - 1;
+        excep_buffer.name_length =
+            pko_copy_exception_name(&excep_buffer, module_name,
+                                    sizeof("unknown") - 1);
     } else {
-        len = 0;
-        while (len < 127 && module_name[len] != '\0')
-            len++;
+        excep_buffer.name_length =
+            pko_copy_exception_name(&excep_buffer, module_name,
+                                    PKO_IOP_EXCEPTION_NAME_BYTES - 1);
     }
-    // Leave room for the header, frame, metadata and terminating NUL.
-
-
-    buffer[0] = 0x0d02beba; // reverse engineering..
-    buffer++;
-
-    memcpy(buffer, frame, sizeof(*frame));
-    buffer += (sizeof(exception_frame_t) / 4);
 
-    buffer[0] = r_epc;
-    buffer[1] = len;
-    buffer += 2;
-    memcpy(buffer, module_name, len);
-    ((char *)buffer)[len] = '\0';
+    excep_buffer.command = 0x0d02beba;
+    memcpy(&excep_buffer.frame, frame, sizeof(*frame));
+    excep_buffer.relative_epc = r_epc;
 
-    pkoSendSifCmd(PKO_RPC_IOPEXCEP, excep_buffer, BUFFER_SIZE);
+    pkoSendSifCmd(PKO_RPC_IOPEXCEP, &excep_buffer, sizeof(excep_buffer));
 }
 
 

iop/exception_packet.h

@@ -0,0 +1,33 @@
+#ifndef PS2LINK_EXCEPTION_PACKET_H
+#define PS2LINK_EXCEPTION_PACKET_H
+
+/* The EE receiver expects the module name at word 43 of the SIF packet. */
+#define PKO_IOP_EXCEPTION_NAME_BYTES 128
+
+typedef struct {
+    u32 command;
+    exception_frame_t frame;
+    u32 relative_epc;
+    u32 name_length;
+    char module_name[PKO_IOP_EXCEPTION_NAME_BYTES];
+} pko_iop_exception_packet_t;
+
+/* Always leave a NUL inside the fixed-size packet name field. */
+static unsigned int pko_copy_exception_name(pko_iop_exception_packet_t *packet,
+                                            const char *src,
+                                            unsigned int source_limit)
+{
+    unsigned int len = 0;
+
+    if (source_limit >= PKO_IOP_EXCEPTION_NAME_BYTES)
+        source_limit = PKO_IOP_EXCEPTION_NAME_BYTES - 1;
+
+    while (len < source_limit && src[len] != '\0') {
+        packet->module_name[len] = src[len];
+        len++;
+    }
+    packet->module_name[len] = '\0';
+    return len;
+}
+
+#endif

tests/exception_packet.c

@@ -0,0 +1,63 @@
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+
+typedef uint32_t u32;
+typedef struct {
+    u32 epc, cause, badvaddr, sr, regs[32], hi, lo, dcic;
+} exception_frame_t;
+
+#include "../iop/exception_packet.h"
+
+static int check_name(const char *src, unsigned int source_length,
+                      unsigned int expected_length)
+{
+    struct {
+        unsigned char before;
+        pko_iop_exception_packet_t packet;
+        unsigned char after;
+    } data;
+    unsigned int length;
+
+    memset(&data, 0xa5, sizeof(data));
+    length = pko_copy_exception_name(&data.packet, src, source_length);
+    return length == expected_length &&
+           data.packet.module_name[length] == '\0' &&
+           data.before == 0xa5 && data.after == 0xa5 &&
+           memcmp(data.packet.module_name, src, length) == 0;
+}
+
+int main(void)
+{
+    char long_name[256];
+    unsigned int i;
+
+    for (i = 0; i < sizeof(long_name) - 1; i++)
+        long_name[i] = 'X';
+    long_name[sizeof(long_name) - 1] = '\0';
+
+    /* The EE receiver uses the existing wire offsets, including word 43. */
+    if (sizeof(exception_frame_t) != 156 ||
+        offsetof(pko_iop_exception_packet_t, frame) != 4 ||
+        offsetof(pko_iop_exception_packet_t, relative_epc) != 160 ||
+        offsetof(pko_iop_exception_packet_t, name_length) != 164 ||
+        offsetof(pko_iop_exception_packet_t, module_name) != 168 ||
+        sizeof(pko_iop_exception_packet_t) != 296) {
+        fputs("FAIL IOP exception wire layout\n", stderr);
+        return 1;
+    }
+
+    if (!check_name("", 0, 0) || !check_name("unknown", 7, 7) ||
+        !check_name(long_name, sizeof(long_name) - 1, 127)) {
+        fputs("FAIL IOP exception name bounds\n", stderr);
+        return 1;
+    }
+    long_name[123] = '\0';
+    if (!check_name(long_name, 123, 123)) {
+        fputs("FAIL boundary-length module name\n", stderr);
+        return 1;
+    }
+    puts("PASS exception packet layout and bounded module names");
+    return 0;
+}