# DF-0706 — Lifecycle bugs in edge handler cleanup: UAF on replaced edge_sock, missing thread join, soclose+sofree imbalance

## Verdict: NOT REPRODUCED (LATENT — entire ip_fw3_sync.c is dead code on a default kernel)

The finding cites four real defects in `sys/net/ipfw3_basic/ip_fw3_sync.c`:

| # | Bug | Mechanism |
|---|-----|-----------|
| (a) | UAF on replaced `edge_sock` | `edge_socket_handler` caches `so` at :195 for the receive loop but uses `fw3_sync_ctx.edge_sock` for cleanup at :227-228; if `edge_conf` replaced the global meanwhile, cleanup hits the wrong socket and leaks the original |
| (b) | `soclose`+`sofree` imbalance | `edge_stop` calls `soclose` (:355); handler calls `sofree` (:228); only one `soreference` was added at `edge_start` :259 → two ref-drops for one ref-add |
| (c) | `MOD_UNLOAD` never joins `edge_td` | `ip_fw3_sync_modevent` at :480-484 sets `running=0`, calls `soclose`, and `edge_td = NULL` — but the handler thread keeps running until its receive returns; if the module is unmapped first, the handler executes unmapped code |
| (d) | `MOD_UNLOAD` never clears `ipfw_sync_send_state_prt` | after unload the global still points at `ip_fw3_sync_send_state` which is now unmapped memory |

All four are real defects **in code that can never execute** on a default
kernel. The entire `ip_fw3_sync.c` file is dead code, for two independent
reasons that each suffice on their own:

### Why the code paths are unreachable

**(1) `ip_fw3_ctl_sync_ptr` is never assigned.** The IP_FW_SYNC_*
sockopt opcodes are dispatched in `sys/net/ipfw3/ip_fw3.c` at lines
1113-1128 only inside `if (ip_fw3_ctl_sync_ptr != NULL)`. That pointer
is initialised to NULL (line 133) and **never assigned anywhere in the
tree**:

```
$ grep -rn 'ip_fw3_ctl_sync_ptr =' sys/
sys/net/ipfw3/ip_fw3.c:133:ip_fw_ctl_t	*ip_fw3_ctl_sync_ptr = NULL;
```

So every `IP_FW_SYNC_*` opcode silently falls through with `error = 0`
(setsockopt returns success with no effect). Confirmed in `run.log`.

**(2) `ip_fw3_sync_modevent` is never called.** The companion function
pointer `ipfw_sync_send_state_prt` (and `ipfw_sync_install_state_prt`)
would be set by `ip_fw3_sync_modevent(MOD_LOAD)` (ip_fw3_sync.c:518-520).
But `ip_fw3_sync_modevent` is **never invoked by anyone** — the
ipfw3_basic module hook (`ipfw3_basic_modevent`, ip_fw3_basic.c:657-658)
calls only `ip_fw3_state_modevent` and `ip_fw3_table_modevent`,
skipping sync entirely:

```
$ grep -rn 'ip_fw3_sync_modevent\|ip_fw3_state_modevent\|ip_fw3_table_modevent' sys/
sys/net/ipfw3_basic/ip_fw3_basic.c:657:    ip_fw3_state_modevent(type);
sys/net/ipfw3_basic/ip_fw3_basic.c:658:    ip_fw3_table_modevent(type);
sys/net/ipfw3_basic/ip_fw3_sync.h:86: void ip_fw3_sync_modevent(int type);
sys/net/ipfw3_basic/ip_fw3_sync.c:514: ip_fw3_sync_modevent(int type)
sys/net/ipfw3_basic/ip_fw3_state.c:712: ip_fw3_state_modevent(int type)
sys/net/ipfw3_basic/ip_fw3_table.c:616: ip_fw3_table_modevent(int type)
```

`state` and `table` are called from `ipfw3_basic_modevent`; `sync` is
declared and defined but **not called from there**. So `MOD_LOAD` of
ipfw3_basic never sets `ipfw_sync_send_state_prt`, and `MOD_UNLOAD`
never runs the cleanup code at all.

### Empirical confirmation

`run.log` shows every `IP_FW_SYNC_*` opcode returning `rc=0 errno=0`
(silent fall-through) after `kldload ipfw3.ko` and `kldload ipfw3_basic.ko`
on the default `with-src` kernel (#0 build, INVARIANTS ON):

```
[*] loading ipfw3.ko + ipfw3_basic.ko
[*] running as: uid=0 euid=0
[fire IP_FW_SYNC_EDGE_CONF       opcode=84 plen=8] rc=0 errno=0
[fire IP_FW_SYNC_EDGE_START      opcode=85 plen=4] rc=0 errno=0
[fire IP_FW_SYNC_EDGE_STOP       opcode=86 plen=4] rc=0 errno=0
[fire IP_FW_SYNC_CENTRE_CONF     opcode=89 plen=4] rc=0 errno=0
[fire IP_FW_SYNC_CENTRE_START    opcode=90 plen=4] rc=0 errno=0
[fire IP_FW_SYNC_SHOW_STATUS     opcode=83 plen=4] rc=0 errno=0
[fire IP_FW_SYNC_SHOW_CONF       opcode=82 plen=64] rc=0 errno=0
[+] All opcodes returned without panic.
```

If the buggy paths were live, EDGE_CONF would `socreate()` an `edge_sock`,
EDGE_START would `soreference()` + `kthread_create` the handler thread,
and EDGE_STOP would exercise the soclose+sofree imbalance. None of that
happens — the dispatcher returns success without invoking any sync code.

`MOD_UNLOAD` of ipfw3_basic completes without invoking
`ip_fw3_sync_modevent` (proven by `grep`), so neither (c) nor (d) can
fire — the conditions they require (`edge_td` exists / `ipfw_sync_send_state_prt`
is set) never arise.

### Conclusion

Consistent with DF-0704 and DF-0705 (which both analysed other defects in
this same file and reached the same dead-code conclusion): the cited bugs
are real defects in the source code but cannot be triggered on a default
kernel because the entire ipfw3_sync subsystem is unwired. Marked
`not_reproduced` — `impact=none`, latent.

## Suggested fix

`fix.diff` is a defensive, in-place fix for all four cited bugs. Because
the code is currently dead, the fix cannot be runtime-tested on the
default kernel (no trigger path exists); instead the diff is validated
for `git apply --check` and compiles cleanly. The day someone wires
`ip_fw3_sync_modevent(type)` into `ipfw3_basic_modevent` (a one-line
addition) and assigns `ip_fw3_ctl_sync_ptr`, the fixes are already in
place:

- (a) `edge_socket_handler` now uses the cached `so` for cleanup too
- (b) `edge_stop` no longer calls `soclose`; it signals the handler via `soshutdown` and `kthread_join`s `edge_td`, leaving the single ref-drop to the handler's `sofree`
- (c) `ip_fw3_sync_modevent(MOD_UNLOAD)` now `soshutdown`s the socket, `kthread_join`s `edge_td`, and only then frees resources
- (d) `MOD_UNLOAD` clears `ipfw_sync_send_state_prt = NULL` and `ipfw_sync_install_state_prt = NULL` first, before any other teardown

The fix also picks up DF-0704's per-type `m_len` validation in the
handler as a bonus (it touches the same function). That hardens the
receive loop against short datagrams if the code is ever enabled.

The fix is **defensive only** — it does not enable the dead code (that
is upstream's call) but ensures the bugs are closed if/when the code is
hooked up.