[PATCH] agent: fix a double free in agent message handling

2014-05-14 Thread Hannu Mallat
It could happen that cleaning up the pending message in
agent_receive_message() would as a side effect clear the last
reference to the service via driver context unref function pointer.
Service cleanup would then call connman_agent_cancel(), which would
try to clean up the pending message for the second time as the pending
pointer was still non-NULL.

Fixed by decoupling the pointer to the pending request from the agent
structure before calling agent_request_free().
---
 src/agent.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/agent.c b/src/agent.c
index b9b8dd9..37cf524 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -112,6 +112,17 @@ static void agent_request_free(struct 
connman_agent_request *request)
g_free(request);
 }
 
+static void agent_finalize_pending(struct connman_agent *agent,
+   DBusMessage *reply)
+{
+   struct connman_agent_request *pending = agent-pending;
+   if (pending) {
+   agent-pending = NULL;
+   pending-callback(reply, pending-user_data);
+   agent_request_free(pending);
+   }
+}
+
 static void agent_receive_message(DBusPendingCall *call, void *user_data);
 
 static int agent_send_next_request(struct connman_agent *agent)
@@ -146,9 +157,7 @@ static int agent_send_next_request(struct connman_agent 
*agent)
return 0;
 
 fail:
-   agent-pending-callback(NULL, agent-pending-user_data);
-   agent_request_free(agent-pending);
-   agent-pending = NULL;
+   agent_finalize_pending(agent, NULL);
return -ESRCH;
 }
 
@@ -191,12 +200,9 @@ static void agent_receive_message(DBusPendingCall *call, 
void *user_data)
send_cancel_request(agent, agent-pending);
}
 
-   agent-pending-callback(reply, agent-pending-user_data);
+   agent_finalize_pending(agent, reply);
dbus_message_unref(reply);
 
-   agent_request_free(agent-pending);
-   agent-pending = NULL;
-
err = agent_send_next_request(agent);
if (err  0  err != -EBUSY)
DBG(send next request failed (%s/%d), strerror(-err), -err);
@@ -456,9 +462,7 @@ static void cancel_all_requests(struct connman_agent *agent)
if (agent-pending-call)
send_cancel_request(agent, agent-pending);
 
-   agent-pending-callback(NULL, agent-pending-user_data);
-   agent_request_free(agent-pending);
-   agent-pending = NULL;
+   agent_finalize_pending(agent, NULL);
}
 
for (list = agent-queue; list; list = list-next) {
@@ -521,12 +525,7 @@ void connman_agent_cancel(void *user_context)
if (agent-pending-call)
send_cancel_request(agent, agent-pending);
 
-   agent-pending-callback(NULL,
-   agent-pending-user_data);
-
-   agent_request_free(agent-pending);
-
-   agent-pending = NULL;
+   agent_finalize_pending(agent, NULL);
 
err = agent_send_next_request(agent);
if (err  0  err != -EBUSY)
-- 
1.8.5.3

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] agent: fix a double free in agent message handling

2014-05-14 Thread Patrik Flykt
On Wed, 2014-05-14 at 09:12 +0300, Hannu Mallat wrote:
 It could happen that cleaning up the pending message in
 agent_receive_message() would as a side effect clear the last
 reference to the service via driver context unref function pointer.
 Service cleanup would then call connman_agent_cancel(), which would
 try to clean up the pending message for the second time as the pending
 pointer was still non-NULL.
 
 Fixed by decoupling the pointer to the pending request from the agent
 structure before calling agent_request_free().

Applied, thanks!

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman