[PATCH 0/2] Fix two crashes in the agent request path

[PATCH 0/2] Fix two crashes in the agent request path

[Inti Manuel Yabar-Pagaza]

Both of these turn up in one scenario: an 802.1X network whose
provisioning file leaves a credential out, so the daemon has to ask the
agent for it, and the request is not answered before the 120 s timeout.
They are independent bugs in independent components, but the first is
what made the second easy to reach, so they are sent together.

The client bug has been latent since 2019 and the daemon bug since
2021; both are present at d003d0e (Release 3.12) by inspection.

Patch 1 - iwctl aborts with "free(): double free detected in tcache 2".
display_agent_prompt_release() destroys the stdin l_io without clearing
the file-static pointer, and the creation sites are guarded by
"if (!io)", so the second prompt of a username+password request installs
a read handler on freed memory. A side effect is that the credentials
are never sent at all, so the user sees a two minute hang rather than an
authentication failure.

Patch 2 - iwd segfaults in agent_finalize_pending(). request_timeout()
does not clear agent->pending_id, so after a timeout the daemon holds a
stale id with an empty request queue; when the agent's name drops off
the bus, agent_disconnect() pops NULL off that queue and dereferences
it. Any agent that exits after a request timeout reproduces this,
whether or not it hit patch 1's bug.

Both were verified at runtime by building the same tree patched and
unpatched and running the identical sequence against each: the stock
binaries abort and segfault respectively, the patched ones complete the
prompt and stay up. Tested on iwd 3.12 with ell 0.83, kernel 7.1.4,
MediaTek MT7925 (mt7925e), PEAP/MSCHAPv2.

Heads up that the analysis, patches and commit messages here are largely
AI-generated (Claude), reviewed and tested by me.

Inti Manuel Yabar-Pagaza (2):
  client: fix double free in agent prompts
  agent: clear pending_id on request timeout

 client/display.c | 2 ++
 src/agent.c      | 3 +++
 2 files changed, 5 insertions(+)

-- 
2.55.0

[PATCH 1/2] client: fix double free in agent prompts

[Inti Manuel Yabar-Pagaza]

display_agent_prompt_release() destroys the stdin l_io but leaves the
file-static pointer set. Both creation sites are guarded by "if (!io)",
so it is never recreated and any later use touches freed memory.

RequestUserNameAndPassword is the only request that prompts twice, so
the password prompt installs a read handler on the destroyed io. The
input is never read and no reply is sent, and the Cancel handler or the
exit path then destroys the same io again:

  free(): double free detected in tcache 2

Clear the pointer at both destroy sites. l_io_destroy(NULL) is a no-op.

Fixes: df32279a31c3 ("client: Enable non-interactive mode support for agent prompts")
---
 client/display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/client/display.c b/client/display.c
index f0f3282a..103e4769 100644
--- a/client/display.c
+++ b/client/display.c
@@ -860,6 +860,7 @@ void display_agent_prompt_release(const char *label)
 	if (!command_is_interactive_mode()) {
 		rl_callback_handler_remove();
 		l_io_destroy(io);
+		io = NULL;
 
 		return;
 	}
@@ -975,6 +976,7 @@ void display_exit(void)
 	rl_callback_handler_remove();
 
 	l_io_destroy(io);
+	io = NULL;
 
 	l_signal_remove(window_change_signal);
 
-- 
2.55.0

[PATCH 2/2] agent: clear pending_id on request timeout

[Inti Manuel Yabar-Pagaza]

request_timeout() cancels the outstanding call but leaves
agent->pending_id set. agent_finalize_pending() pops the last request
and agent_send_next_request() returns early on the empty queue, so the
stale id survives with nothing queued.

When the agent later drops off the bus, agent_disconnect() tests that
id and calls agent_finalize_pending() again, which pops NULL off the
empty queue and dereferences it:

  #0  agent_finalize_pending (agent=0x..., reply=0x0) at src/agent.c:187
  #1  agent_disconnect (...) at src/agent.c:511
  #2  _dbus_name_cache_notify (...) at ell/dbus-name-cache.c:188
  #4  name_owner_changed_cb (...) at ell/dbus.c:861

agent_receive_reply() and agent_request_cancel() already clear
pending_id in this situation. Do the same on timeout, and guard
agent_finalize_pending() against an empty queue.

Fixes: d04ab5ad96b4 ("agent: call back even if agent disconnects")
---
 src/agent.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/agent.c b/src/agent.c
index 0f718b87..5adee5dc 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -183,6 +183,8 @@ static void agent_finalize_pending(struct agent *agent,
 	}
 
 	pending = l_queue_pop_head(agent->requests);
+	if (!pending)
+		return;
 
 	switch (pending->type) {
 	case AGENT_REQUEST_TYPE_PASSPHRASE:
@@ -230,6 +232,7 @@ static void request_timeout(struct l_timeout *timeout, void *user_data)
 	struct agent *agent = user_data;
 
 	l_dbus_cancel(dbus_get_bus(), agent->pending_id);
+	agent->pending_id = 0;
 
 	send_cancel_request(agent, -ETIMEDOUT);
 
-- 
2.55.0