Why accel-ppp needs to be rewritten in Rust — evidence from our own git history #369
Replies: 1 comment 4 replies
|
Just a few random thoughts (from someone with experience in C but not in Rust - not against Rust, but simply haven't learned it yet) - part of the problem is also high complexity, trying to do many things and support many different protocols instead of the simpler "do one thing and do it well". |
Uh oh!
There was an error while loading. Please reload this page.
I'd like to make the case for rewriting accel-ppp in Rust. This is not a "rewrites are fashionable" post. Everything below comes from our own repository: ~2,400 commits between 2009 and 2026, ~80,000 lines of C, and commit hashes you can check yourself. The short version: for 17 years, a huge share of our engineering time has gone into fixing the same handful of bug classes that C makes trivially easy to write and hard to find. Rust makes these classes impossible by construction.
1. What the history actually says
Out of 2,403 commits, 719 have "fix" in the title. That's one commit in three. Now filter by the bug class named in the title (a strict undercount - it only catches what the author happened to write in one line):
That's ~130 commits whose headline is a memory-safety or data-race bug - and the titles undersell it badly, because single commits fix batches of such bugs.
62f77400("Fixed some errors found by valgrind and pvs-studio", 2021) alone fixes, by PVS-Studio's count: uninitialized pointers, null dereferences, memory leaks, a function that exits while holdingpthread_spin_unlock, and ~40 instances ofvoid *arithmetic. Every one of those reached users before an analyzer spotted it.Nobody in this project has ever been careless. The same names - Dmitry, Themiron, Guillaume Nault, Denis, nuclearcat - fix these things fast and well. The point is that C lets careful people ship these bugs anyway, and the project pays for it forever.
2. Case files
Here are the ones I find most instructive.
2.1. Remotely triggerable memory corruption in L2TP (2020)
2324bcd5- l2tp: fix RCE through buffer overflow. Insufficient validation of the L2TP header length and AVP length allowed a remote, unauthenticated packet to overflow a buffer - reported by external security researchers (Chaitin Security Research Lab). The same commit also fixed LE/BE compatibility: the header used C bitfields, whose ordering is implementation-defined, so the parser simply misread every field on big-endian machines.Read that again: one bug, three C-specific root causes - manual length checking that was one
ifshort, bitfields whose layout the language refuses to define, and endianness handled by hand.2.2.
uint8_t *calling[254](2025)2bbf451b- l2tp: fix buffer overflow and type errors in Calling/Called Number handling. The declarationdeclares an array of 254 pointers, not 254 bytes. And since L2TP AVP values can be up to 1017 bytes, any peer sending a long Calling-Number overflowed the stack buffer. This landed in 2025 - fifteen years after the L2TP module was written, caught not by fuzzing but by someone reading the diff of a previous change. One stray asterisk, and the type system of C - which famously has a type system - says nothing.
2.3. RADIUS attribute parsing (2021)
d4cb8972- buffer overflow when a RADIUS packet contains an invalid attribute length with attribute typesipv4addr,ipv6addr,ipv6prefixorifid. Again: remote, again: unauthenticated, again: hand-rolledwhileloop over TLVs where one attribute-type branch forgot the length check. And again in 2026 in the same subsystem family:22c61e8e, wheresplit()returning 0 madedict_loadreadptr[-1]- with the commit message noting, verbatim: "Triggering crash is compiler dependent, it might not happen now, but a bit different compiler, flags, and it might crash on load." That sentence is the entire argument against C in miniature: correctness that depends on the phase of the compiler's mood is not correctness.2.4. Use-after-free: a recurring subscription, not an event
Look at the dates in this series:
1744cf7d(2014) - l2tp: use-after-free by tunnel context thread3fa9e6ad(2016) - ppp_auth: possible use-after-free22b24dc6(2018) - ppp: use-after-free inppp_auth_failed()c6667801- triton: use-after-free intimer.c7bd6843c(2026) - radius: "rad_req_free might free server in theory, and s will be invalid"2ba4437b(2026) - pppoe: the mac-filter error path frees the entry and then keeps writing to it and links it into the listTwenty-plus years of tooling, sanitizers in CI, and senior maintainers - and we were fixing use-after-free this year, in a code path triggered by a malformed line in a config file. In Rust this bug does not compile: the borrow checker rejects "free it, then keep using it" as a matter of grammar, not discipline.
2.5. When the optimizer is your enemy
287adbfc(2017) - triton crashed because GCC deleted thealloca()call entirely on any optimization level (the result was used indirectly, and the call is not observable in C's abstract machine). So the project crashed not because the code was wrong-looking, but because C's undefined-behavior rules license the compiler to remove code it "proves" unreachable. The same family:2224396d, where signed integer overflow (undefined behavior in C) silently corrupted DHCPv4 address handling, and the bitfield/endianness bug from 2.1. Rust has noalloca, defines integer overflow semantics (checked in debug, wrapping-by-explicit-choice in release), and gives youu16::from_be_bytesinstead of bitfield prayers.2.6. Uninitialized memory, on the wire
29b90105andc3710b6b- IPCP negotiated DNS and NBNS addresses containing uninitialized stack memory and handed them to peers.62f77400contains a valgrind trace of uninitialized bytes going out throughsendmsg(). In C, "declare a struct, fill in most of the fields" compiles and runs and mostly works. In Rust there is no way to observe uninitialized memory through safe code, period.2.7. The concurrency tax: triton
This is the systemic one. Accel-ppp's event framework, triton, is a hand-rolled world of contexts, threads, wakeup lists, and manual
pthreadlocking. Its cost is visible across the whole history:5ee3cf66- shaper: fixed many race conditions (plural, in one commit)c709a126(2018) - theschedule()/wakeup()race, where a wakeup could "write to invalid memory" and crash the daemon - fixed by redesigning the wakeup protocol, and the commit message candidly explains the API still must be paired and not nested by every caller, foreverfd4bd103,0bf9ca63,c7ad0e53,37e2a417,64f65c44,a77f4482,61d91047,80eab99d… the list goes on for 27 titles.A PPPoE/L2TP daemon is a shared mutable state machine with dozens of concurrent sessions plus config reload plus CLI plus logging threads. In C, data races are invisible to the compiler - none of our warnings, sanitizers, or reviews can prove absence of races; they can only fail to find them. Rust's
Send/Syncbounds and ownership model turn "this shared mutable state needs locking" into a compile-time error, andstd::sync+ channels (or async) replace triton's most fragile machinery with primitives that thousands of projects exercise daily.3. The pattern
Notice what all these fixes have in common. In every case:
uint8_t *calling[254]sat there for 15).NULLcheck, one more lock, one more_safelist iterator (aa9e2eefliterally swaps inlist_for_each_safeafter a segfault).Our response so far has been to bolt on detectors - valgrind runs, PVS-Studio, cppcheck, ASan/UBSan in CI. These tools are valuable and I don't want to drop them. But every one of them finds instances after the bug exists, never classes before. The rust toolchain finds the class, at compile time, in every future commit, for free:
calling[254]typo doesn't compileDropruns on every exit path, including error paths like2ba4437bSend/Sync; shared mutability requires synchronization the compiler checksalloca, signed overflow, bitfields,ptr[-1])alloca; overflow semantics defined; no raw bitfields; slicing can't go negativefrom_be_bytes/to_be_byteseverywhere4. Beyond memory safety
A rewrite buys more than the borrow checker:
cargo-fuzzgives per-target coverage-guided fuzzing in a few lines; today standing up the equivalent for our C parsers is a project of its own. Fuzzing safe Rust finds logic bugs, not memory corruption - a completely different (and much cheaper) triage experience.configure/CMake dance across five OS containers withcargo buildandcargo vendor. Compareaccel-cmd: on this very branch, 774 lines of legacy C (legacy/accel_cmd.c) became ~750 lines of Rust including a real test suite (src/tests.rs) that the C version never had. Same protocol, same features, safer and tested - that's the proof of concept, and it's already in-tree.Optioninstead of sentinelNULLs) generate code in the same class as C. For a daemon whose hot path is packet parse/mutate/serialize, the difference is noise; the wins - fearless refactoring of session state machines - are not.5. Step zero: build the oracle first - massive AI-generated Python tests
Here is the part I want to insist on, because it changes the risk profile of the whole endeavor: before rewriting anything, we should generate a very large black-box test suite with AI assistance - in Python, not as C unit tests.
The reasoning:
accel-cmd show sessions. The same suite, unchanged, runs against the C daemon today and against the Rust daemon after each migration step. It is the oracle that makes "the rewrite behaves identically" a checkable claim instead of a hope.tests/is a pytest-based black-box suite with veth setup, dependency ordering and gcovr coverage - 18 test files today. That's the seed, not the finish line. The realistic target is to scale it by an order of magnitude or two, and that is exactly the kind of work AI is best at: mechanically enumerating scenario matrices (auth success/failure/timeout/reject, malformed and truncated packets per protocol, config permutations, session lifecycle races, reload-under-load) and writing the corresponding pytest code against an existing, well-defined harness.In short: the AI era lets us buy, cheaply, the thing every rewrite project historically lacked - an exhaustive behavioral snapshot of the old system. We should buy it before we need it.
6. The "Rust is harder to write" objection has expired
The classic argument against Rust for a project like this: steep learning curve, borrow-checker friction, slower feature velocity than "just write the C." That argument made sense when every line was typed by hand. It makes less sense every year, and I think it's already false:
Resultplumbing and iterator gymnastics.if-short length checks and free-then-use error paths we saw in section 2 - but at a new scale and speed, and polished enough to look right in review. An AI writing Rust produces code the borrow checker still refuses to compile if it's wrong about ownership, lifetimes, or data races. In other words, AI amplifies the case for Rust rather than weakening it: as code generation gets cheaper, machine-checked safety guarantees get more valuable, not less - because the bottleneck moves from "writing code" to "being sure about code," and being sure is precisely what C's toolchain cannot offer and Rust's compiler does by default.So the honest comparison in 2026 is not "expert C velocity vs. slower Rust velocity." It is "AI-assisted C, where the memory-safety review burden stays on humans forever" vs. "AI-assisted Rust, where the compiler carries most of that burden for free."
7. The honest objections
drivers/stay C, of course - that's the kernel's world, not ours.8. Proposal
accel-cmd/build.rs) and CI for mixed C/Rust.I'm not saying this is small. I'm saying the alternative is
2bbf451bagain in 2035 - another one-line typo in a hand-rolled parser, fifteen years latent, holding a remotely exploitable stack overflow, found by luck. The git history gives us the data; the data says the bug classes never stop. The only thing that stops a bug class is a compiler that refuses it.All reactions