# DF-0489 — "NA handler leaks route refcount" — NOT REPRODUCED (FALSE POSITIVE)

## Verdict: NOT REPRODUCED — FALSE POSITIVE

The finding claims `nd6_na_input` leaks one route refcount per received NA
because `nd6_lookup` acquires a referenced route (`route.c:276` `rt->rt_refcnt++`)
and `nd6_na_input` never calls `rtfree()`. **The claim is false.** The reviewer
traced the `++` at `route.c:276` but **missed `nd6.c:929` `rt->rt_refcnt--`**,
which sits inside `nd6_lookup` itself and unconditionally undoes the `++` before
the route is returned. The route handed to `nd6_na_input` therefore carries a
**net-zero reference change** — there is nothing to free and no leak.

## The guard the reviewer missed (path:line)

`nd6_na_input` (`sys/netinet6/nd6_nbr.c:734`):
```c
rt = nd6_lookup(&taddr6, 0, ifp);   /* create = 0 */
```

`nd6_lookup` (`sys/netinet6/nd6.c:859`) with `create = 0`:
```c
/* line 873 */ rt = rtpurelookup((struct sockaddr *)&sin6);
               /*  -> _rtlookup -> route.c:276: rt->rt_refcnt++   (+1)  */
...
/* line 929 */ rt->rt_refcnt--;      /* UNCONDITIONAL, before return   (-1)  */
...
/* line 940 */ return (rt);          /* route returned NET-ZERO ref change */
```

`route.c:276` (`++`, inside `_rtlookup`) is **exactly balanced** by `nd6.c:929`
(`--`, inside `nd6_lookup`). The `--` at line 929 is reached on **every** path
where `nd6_lookup(create=0)` returns a non-NULL route:
- lookup succeeds, route has `RTF_LLINFO` → skip the `if(!rt)` block → line 929 `--`.
- lookup succeeds, route lacks `RTF_LLINFO`, `create=0` → the inner `if(create)`
  is skipped → `if(!rt)` is false → line 929 `--`.

So the route returned to `nd6_na_input` has **no holding reference**.

`git blame` confirms `nd6.c:929` is in the **same commit** (`6cc80ee9`) as the
rest of the audited file — it was not recently added; the reviewer simply did not
trace into `nd6_lookup` far enough.

## Corroborating evidence: no other caller frees the result

Every other caller of `nd6_lookup(addr, 0, ...)` in the tree also does **not**
call `rtfree()` on the result — confirming the convention that `create=0` returns
an unreferenced route:

| caller | file:line | rtfree? |
|--------|-----------|---------|
| `nd6_na_input` | nd6_nbr.c:734 | no |
| `icmp6` RA build | icmp6.c:2493 | no |
| `nd6_is_addr_neighbor` | nd6.c:1137 | no |
| `nd6_cache_lladdr` | nd6.c:1649/1710 | no |
| default-router select | nd6_rtr.c:603 | no |
| prefix-router select | nd6_rtr.c:1148 | no |

If `nd6_lookup(create=0)` returned a referenced route, **all** of these would
leak — the kernel would be unable to keep any neighbor route alive. It does not,
because it returns an unreferenced route.

## Empirical verification

Two RX-injection mechanisms were attempted on the `#0` unpatched kernel; both
confirmed the finding's claimed leak does **not** manifest (Refs stayed flat):

1. **BPF write injection** (`poc_df0489.c`): injected 5000 NA frames. BPF writes
   call `ifp->if_output` (`sys/net/bpf.c:598`) — i.e. **TX/output**, not RX/input.
   The NAs went out the interface, never reaching `nd6_na_input` (icmp6 Input
   histogram showed zero neighbor-adverts). Route `Refs` stayed at 0.

2. **netgraph ng_ether RX injection** (`poc_df0489_ng.c`): loaded `ng_ether` +
   `ng_socket`. The `NGIOCSETNAME` ioctl needed to name the injection socket node
   returned `ENOTSUP` on this build, so the `vtnet0:lower` (RX) hook could not be
   wired up; all data writes returned -1.

The empirical RX path was not exercisable with the available guest mechanisms,
but the **source trace alone is definitive**: the `--` at `nd6.c:929` is the guard
that closes the claimed leak. This is Phase-4 case (a): "the bug is not real; here
is the check at path:line that prevents it."

## Why the finding's reasoning was plausible but wrong

The finding correctly observed:
- `route.c:276` does `rt->rt_refcnt++` (TRUE).
- `nd6_na_input` (nd6_nbr.c:601-907) has no `rtfree()` (TRUE).

But it failed to trace **inside** `nd6_lookup`, where the balancing
`rt->rt_refcnt--` at `nd6.c:929` lives. The `++` and `--` are in two different
functions (`_rtlookup` and `nd6_lookup`), which made the imbalance easy to miss
in a single-function scan. The net effect across the call chain is zero.

## Secondary observation (separate from this finding)

The unconditional `--` at `nd6.c:929` is itself a mild correctness oddity: when
`create=0` and the matched route is not a neighbor for `ifp`,
`ND6_RTENTRY_IS_NEIGHBOR` returns NULL at line 938 **after** already decrementing
the refcount — so a non-neighbor lookup silently drops a reference it never added
net (the `++` from `_rtlookup` was balanced, then `--` runs, net -1). This is a
different, latent issue (potential spurious refcount decrement on non-neighbor
lookups), **not** the leak this finding describes, and it is out of scope here.
No code change is recommended for this finding.

## Recommended fix

**No code change needed — false positive.** The claimed refcount leak does not
exist because `nd6_lookup` (`nd6.c:929`) balances the reference acquired by
`_rtlookup` (`route.c:276`). `nd6_na_input` correctly does not call `rtfree()`
because it was never handed a referenced route.
