Destroy mempool on module exit to fix memory leak

[Jordymalone]

khttpd_init() preallocates POOL_MIN_NR buffers when creating http_buf_pool, but khttpd_exit() never releases the pool, so the buffers stay allocated after every module unload until reboot. Adding mempool_destroy() on the exit path releases them and resolves the leak.

---

Summary by cubic

Free the preallocated HTTP buffer mempool on module unload to fix a memory leak. Adds mempool_destroy(http_buf_pool) to khttpd_exit(), releasing the POOL_MIN_NR buffers from khttpd_init() so they don’t persist across unloads.

^{Written for commit 46e44e3. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="main.c">

<violation number="1" location="main.c:187">
P1: Destroying `http_buf_pool` on the exit path resolves the leak only if no worker thread is still using the pool at that moment. However, `khttpd_exit()` only stops the daemon thread (`kthread_stop(http_server)`); the per-connection `http_server_worker` kthreads spawned in `http_server_daemon()` are never tracked or stopped, so with an active connection a worker can still be blocked in recv holding a `mempool_alloc`'d buffer when `mempool_destroy()` frees the pool and runs `http_buf_free` on each element. The worker's later `mempool_free(buf, http_buf_pool)` then touches freed memory — converting the leak fix into a use-after-free/double-free (likely a crash) whenever unload happens with an open connection. Consider tracking all worker kthreads, stopping them (and draining/returning their buffers) before destroying the pool, so the destroy only runs once no in-flight buffers exist.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Destroying http_buf_pool on the exit path resolves the leak only if no worker thread is still using the pool at that moment. However, khttpd_exit() only stops the daemon thread (kthread_stop(http_server)); the per-connection http_server_worker kthreads spawned in http_server_daemon() are never tracked or stopped, so with an active connection a worker can still be blocked in recv holding a mempool_alloc'd buffer when mempool_destroy() frees the pool and runs http_buf_free on each element. The worker's later mempool_free(buf, http_buf_pool) then touches freed memory — converting the leak fix into a use-after-free/double-free (likely a crash) whenever unload happens with an open connection. Consider tracking all worker kthreads, stopping them (and draining/returning their buffers) before destroying the pool, so the destroy only runs once no in-flight buffers exist.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At main.c, line 187:

<comment>Destroying `http_buf_pool` on the exit path resolves the leak only if no worker thread is still using the pool at that moment. However, `khttpd_exit()` only stops the daemon thread (`kthread_stop(http_server)`); the per-connection `http_server_worker` kthreads spawned in `http_server_daemon()` are never tracked or stopped, so with an active connection a worker can still be blocked in recv holding a `mempool_alloc`'d buffer when `mempool_destroy()` frees the pool and runs `http_buf_free` on each element. The worker's later `mempool_free(buf, http_buf_pool)` then touches freed memory — converting the leak fix into a use-after-free/double-free (likely a crash) whenever unload happens with an open connection. Consider tracking all worker kthreads, stopping them (and draining/returning their buffers) before destroying the pool, so the destroy only runs once no in-flight buffers exist.</comment>

<file context>
@@ -184,6 +184,7 @@ static void __exit khttpd_exit(void)
     send_sig(SIGTERM, http_server, 1);
     kthread_stop(http_server);
     close_listen_socket(listen_socket);
+    mempool_destroy(http_buf_pool);
     pr_info("module unloaded\n");
 }
</file context>

[Jordymalone]

Fair point about the pool use-after-free. That part is indeed introduced by this patch. The scenario is already fatal without it though, since workers are never tracked and kthread_run() takes no module reference. A worker blocked in recv would resume into freed module code once rmmod completes, so unloading with an open connection crashes either way. The leak fix itself is still needed for the normal unload path.

[Jordymalone]

@jserv would you prefer to keep this patch and handle worker tracking in a follow up, fold both into this PR, or close this one?

[jserv]

@jserv would you prefer to keep this patch and handle worker tracking in a follow up, fold both into this PR, or close this one?

Fold here as the changes should be reasonably small.