Comfy-Org / Comfy-Org/comfy-aimdo

Fix CUDA 13.0 crash: replace Detours with manual hooks + fix RIP-relative overflow via near allocation

Open
#75 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
C
Stars
67
Forks
39
Avg merge
1d 25m
Merged PRs (30d)
10

Description

# comfy-aimdo CUDA 13.0 Hook Fix — Full Report

**Author:** Community contributor (openwork)
**Date:** 2026-07-23
**Test duration:** 7+ days of daily use, no crashes or CUDA errors

---
**Since I'm not a professional programmer and the code has been modified by AI, the fixes may only apply to my own case. This post is intended solely for sharing ideas.**
---

## Platform

| Component | Version |
|---|---|
| OS | Windows 11 |
| GPU | NVIDIA GeForce RTX 3070 Laptop GPU |
| Driver | 610.62 |
| CUDA | 13.0 |
| PyTorch | 2.10.0+cu130 |
| ComfyUI | v0.28.0-29-g54ca9193 |
| comfy-aimdo | v0.4.10 (commit `gace72abef`) |

---

## Table of Contents

1. [Detours Incompatibility with CUDA 13.0](#1-detours-incompatibility-with-cuda-130)
2. [alloc_near Failure & RIP-Relative Overflow](#2-alloc_near-failure--rip-relative-overflow)
3. [Robustness Fixes](#3-robustness-fixes)

---

## 1. Detours Incompatibility with CUDA 13.0

### Symptom

ComfyUI crashes at startup with an access violation:

```
aimdo: src-win/cuda-detour.c:INFO:aimdo_setup_hooks: installing 6 hooks
aimdo: install_hook_entries: Hook cuMemAlloc_v2 failed 00007FFBAF2B5340
Windows fatal exception: access violation

OSError: exception: access violation writing 0x0000000000000000
File "control.py", line 122 in init_devices
if lib.init(device_array, headroom_array, len(requested)):
```

### Root Cause

`DetourAttach` returns error code 8 (`ERROR_NOT_ENOUGH_MEMORY`) for all 6 hooks. The investigation tree was:

1. **Pre-resolution theory (disproven):** Adding dummy calls to `cuMemAlloc`/`cuMemFree` to force IAT resolution did not help — all IAT entries were already resolved.
2. **JMP-stub theory (disproven):** In CUDA 13.0, `cuGetProcAddress` returns **real function body addresses** inside `nvcuda64.dll` directly, not JMP stubs in `nvcuda.dll`. The function prologue is `48 83 ec 28 81 3d ` (14 bytes) for all 6 targets.
3. **Detours internal failure (confirmed):** Detours Express's `.detourc`/`.detourd` section mechanism and helper-DLL trampoline allocation cannot handle the CUDA 13.0 driver's function layout.
- `DetourAllocateRegionWithinJumpBounds` returns NULL
- `DetourCopyInstruction` fails on the first instruction
- `DetourSetSystemRegionLowerBound`/`UpperBound` are not exported from `detours.lib`

### Resolution

**Abandon Detours entirely. Implement a manual x64 code hook.**

```c
// Absolute indirect JMP: FF 25 00 00 00 00 <8-byte-addr>
// i.e. jmp QWORD PTR [rip+0]
// The 8 bytes following the instruction hold the absolute target address.
static inline void write_abs_jmp(unsigned char *where, void *target) {
where[0] = 0xFF; where[1] = 0x25;
where[2] = 0x00; where[3] = 0x00;
where[4] = 0x00; where[5] = 0x00;
*(void **)(where + 6) = target;
}
```

**Hooking flow:**

1. Read the first 14 bytes of the target function into `saved[]` buffer
2. Allocate a 64-byte trampoline
3. Copy `sub rsp, 0x28` (4 bytes) verbatim to trampoline
4. Replace `cmp [rip+disp32], imm32` with absolute addressing (see Fix 2)
5. Append a 14-byte absolute JMP back to `target+14` at the end of the trampoline
6. Overwrite the target function's first 14 bytes with a 14-byte absolute JMP to the hook function
7. On failure, restore all patched targets from `saved[]` and free trampolines

**Files changed:**
- `src-win/cuda-detour.c` — complete rewrite from Detours-based to manual hooks
- `src-cuda/dispatch.c` — reverted the ineffective IAT pre-resolution patch
- Linker flags — removed `detours.lib`

---

## 2. alloc_near Failure & RIP-Relative Overflow

### Symptom

After Fix 1 was deployed, the hooks would install successfully on some boots but fail on others with ASLR:

```
aimdo: alloc_near: target=00007FFBB3595340 alloc_base=00007FFBB3360000
aimdo: alloc_near: mod_end=00007FFBB523F000
aimdo: alloc_near: failed to find near allocation for target=00007FFBB3595340
```

### Root Cause (4 Layers)

#### Layer 1 — 132 GB distance overflow

`VirtualAlloc(NULL, 64, ...)` returns addresses in the low range (`0x000001...`), while `nvcuda64.dll` loads at high addresses (`0x7FFB...`). The difference is ~132 GB, far exceeding `int32_t` (±2 GB). The `copy_rip_rel_inst` fixup (`src - dst`) truncates, producing a wrong displacement for the `cmp [rip+disp]` instruction — pointing to an unrelated global variable.

The program booted without crashing (the `cmp` likely checks a lazy-init flag that was already set), but during inference the comparison read garbage memory, potentially causing undefined behavior.

#### Layer 2 — VirtualQuery 64 KB alignment bug

The forward scan used 64 KB **align-down**:

```c
at = (unsigned char *)((uintptr_t)at & ~0xFFFFULL); // WRONG
```

After the first hook allocated at `0x...730000`, the next MEM_FREE region started at `0x...731000`. Aligning `0x...731000` **down** produces `0x...730000` (already allocated), so `VirtualAlloc` fails.

**Fix:** forward scan must align **up**; backward scan must align **down**.

#### Layer 3 — 256-iteration cap

The scan loop had an artificial 256-iteration limit. On some ASLR layouts, CUDA + PyTorch load enough DLLs and memory-mapped files to fill the 2 GB neighbourhood with committed pages, requiring more than 256 `VirtualQuery` iterations to exhaust the address range.

**Fix:** replace the iteration cap with address-range bounds (`target ± INT32_MAX`).

#### Layer 4 — No MEM_FREE found at all

After fixing Layers 1-3, the scan log showed:

```
fwd scan=00007FFBB3FAF000 base=00007FFBB3FAF000 state=65536 size=4096 iter=0
```

The only MEM_FREE region after `nvcuda64.dll` was **4 KB** — insufficient for a 64 KB-aligned `MEM_COMMIT | MEM_RESERVE` allocation. The subsequent 2 GB range was entirely `MEM_COMMIT` pages (a dense layout caused by CUDA 13.0 + PyTorch 2.10 + Windows 11 process initialization).

**No amount of VirtualQuery tuning can find a suitable free slot** because one does not exist within the ±2 GB window.

### Resolution

**Abandon RIP-relative fixup entirely. Use absolute addressing.**

```c
// BEFORE: copy + fixup RIP-relative offset (trampoline must be within ±2 GB)
static void copy_rip_rel_inst(unsigned char *dst, unsigned char *src) {
int64_t adj = (int64_t)(src - dst);
*(int64_t *)dst = *(int64_t *)src;
*(int32_t *)(dst + 2) = *(int32_t *)(src + 2) + (int32_t)adj;
*(int32_t *)(dst + 6) = *(int32_t *)(src + 6);
}

// AFTER: absolute addressing (trampoline can be anywhere)
// push rax ; 1 byte (preserve caller's RAX)
// mov rax, ; 10 bytes (load the global address)
// cmp [rax], ; 6 bytes (compare with immediate)
// pop rax ; 1 byte (restore RAX)
static void emit_abs_cmp(unsigned char *dst, unsigned char *src) {
void *global_addr = src + 10 + *(int32_t *)(src + 2);
uint32_t imm32 = *(uint32_t *)(src + 6);
dst[0] = 0x50; // push rax
dst[1] = 0x48; dst[2] = 0xB8; // mov rax, imm64
*(void **)(dst + 3) = global_addr;
dst[11] = 0x81; dst[12] = 0x38; // cmp [rax], imm32
*(uint32_t *)(dst + 13) = imm32;
dst[17] = 0x58; // pop rax
}
```

Since the trampoline no longer needs to be near the target, `alloc_near` becomes trivial:

```c
static void *alloc_near(void *target, size_t size) {
return VirtualAlloc(NULL, size, MEM_COMMIT | MEM_RESERVE,
PAGE_EXECUTE_READWRITE);
}
```

Trampoline layout (36 of 64 bytes used):
```
offset 0: sub rsp, 0x28 ; 4 bytes (copied from target)
offset 4: push rax ; 1 byte
offset 5: mov rax, ; 10 bytes
offset 15: cmp [rax], ; 6 bytes
offset 21: pop rax ; 1 byte
offset 22: jmp [rip+0] ; 6 bytes (FF 25 00 00 00 00)
offset 28: <8-byte target+14> ; absolute address to jump back to
```

**Files changed:**
- `src-win/cuda-detour.c` — `alloc_near` simplified from 147 lines of VirtualQuery scanning to 1 line; `copy_rip_rel_inst` replaced by `emit_abs_cmp`

---

## 3. Robustness Fixes

Three additional issues were found and fixed during code review.

### 3a — RAX Clobber

**Risk:** The original instruction `cmp [rip+disp32], imm32` (`81 3D ...`) does **not** modify any general-purpose register. Our replacement `mov rax, ` overwrites RAX, potentially breaking code that uses the caller's RAX value after the prologue.

**Fix:** Wrap the absolute-addressing sequence with `push rax` / `pop rax` (2 extra bytes, 36 B total trampoline size).

### 3b — Opcode Check Too Permissive

**Risk:**

```c
// BEFORE: only checks Mod + R/M fields, ignores Reg (opcode extension) field
if (s[0] == 0x81 && (s[1] & 0xC7) == 0x05) {
```

`0xC7 = 11000111` only preserves Mod (bits 7:6) and R/M (bits 2:0), masking out Reg (bits 5:3). The `81` opcode's `/digit` selects the operation:
- `/7` = cmp (our target)
- `/0` = add, `/1` = or, `/2` = adc, etc.

This would match `add [rip+...], imm32` just as easily as `cmp`. While all 6 current targets are indeed `81 3D` (cmp), this is an implicit assumption.

**Fix:**

```c
// AFTER: exact match for cmp [rip+disp32], imm32
if (s[0] == 0x81 && s[1] == 0x3D) {
```

`81 3D` = ModRM `00 111 101` = Mod=00, Reg=111(cmp), R/M=101(RIP-relative).

### 3c — VirtualProtect Return Value Not Checked

**Risk:**

```c
DWORD old;
VirtualProtect(target, HOOK_CODE_SIZE, PAGE_EXECUTE_READWRITE, &old); // might fail silently
write_abs_jmp(target, hooks[i].hook_ptr); // crash on unwritable memory
```

If `VirtualProtect` fails (security policy, debugger protection, read-only pages), the subsequent write causes an access violation in an already-partially-patched state.

**Fix:**

```c
if (!VirtualProtect(target, HOOK_CODE_SIZE, PAGE_EXECUTE_READWRITE, &old)) {
log(ERROR, "%s: %s VirtualProtect failed\n", __func__, hooks[i].name);
VirtualFree(tramp, 0, MEM_RELEASE);
goto fail;
}
```

The `fail:` unwind path restores all earlier hooks from `saved[]` and frees their trampolines:

```c
fail:
for (size_t u = 0; u < installed; u++) {
unsigned char *t = (unsigned char *)*hooks[u].target_ptr;
DWORD old;
VirtualProtect(t, HOOK_CODE_SIZE, PAGE_EXECUTE_READWRITE, &old);
memcpy(t, saved[u], HOOK_CODE_SIZE);
FlushInstructionCache(GetCurrentProcess(), t, HOOK_CODE_SIZE);
VirtualProtect(t, HOOK_CODE_SIZE, old, &old);
VirtualFree(*hooks[u].true_ptr, 0, MEM_RELEASE);
*hooks[u].true_ptr = NULL;
}
return false;
```

---

## Final Code State (`src-win/cuda-detour.c`)

The complete file is 153 lines. Key components:

| Component | Lines | Purpose |
|---|---|---|
| `alloc_near` | 10-16 | Simple `VirtualAlloc` — no more scanning |
| `write_abs_jmp` | 21-26 | `FF 25 00 00 00 00 <8-byte>` absolute indirect JMP |
| `emit_abs_cmp` | 40-57 | Absolute-addressing `cmp` with RAX preservation |
| `install_hook_entries` | 65-140 | Main hooking logic with unwind on failure |
| `aimdo_setup_hooks` | 142-149 | Entry point called from `init()` |

Hooks are no longer linked against Detours (`detours.lib` removed from linker input). All hooking is done via `VirtualAlloc` + `VirtualProtect` + raw byte manipulation — zero external dependencies.

## Test Summary

- **Duration:** 7+ consecutive days of daily ComfyUI usage
- **Reboots:** Multiple (ASLR changes the memory layout every time)
- **Result:** `6 hooks installed` on every boot, no crashes, no CUDA errors
- **Workloads tested:** Stable Diffusion image generation (txt2img, img2img, ControlNet, IP-Adapter), LoRA loading, model offloading (comfy-aimdo's primary feature)

Contributor guide

Open the contributing guide

Research direction

Start with src-win/cuda-detour.c, especially aimdo_setup_hooks and install_hook_entries, then review the related revert in src-cuda/dispatch.c. Reproduce initialization with the reported CUDA 13.0 environment and verify that six hooks install across reboots without crashes or CUDA errors; the issue names no automated test.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
ai-infra-agents, operating-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.