From 7702033f4eb40bb70ffb0cbdc1edd7b2efc0903b Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Thu, 24 Jan 2013 16:13:55 +0100 Subject: l2tp: Fix race condition when removing sessions Freeing a session requires four steps: 1-The session asynchronously requests its parent tunnel to remove the session (the tunnel must start the process). 2-The parent tunnel removes its reference to this session, then makes an asynchronous call to the session cleanup function. 3-The session performs its cleanup, frees its memory and asynchronously notifies its parent tunnel. 4-The parent tunnel decrements its sessions counter (so that it knows when it doesn't have any session depending on it). If two requests, A and B, for removing the same session, arrive almost simultaneously, then the following scenario may occur: 1a-The session handles request A and transmits it to its parent tunnel. 2a-The parent tunnel removes its reference to this session and requests the session to perform cleanup. 1b-In the mean time, the session handles request B and transmit it to its parent tunnel. 3a-The session continues to handle request A, cleans up the session and frees memory. 2b-The parent tunnel handles request B: it removes its reference to the session and tells it to perform cleanup. But the session pointer used has just been freed at step 3a. This patch modifies step 1, so that the session transmit the session-id instead of a pointer to the session structure. That way, the tunnel can check if the session has already been deleted (or is pending deletion), without having to manipulate a potentially invalid pointer. Signed-off-by: Guillaume Nault --- accel-pppd/ctrl/l2tp/l2tp.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/l2tp/l2tp.c b/accel-pppd/ctrl/l2tp/l2tp.c index 67825ae..1ad5a81 100644 --- a/accel-pppd/ctrl/l2tp/l2tp.c +++ b/accel-pppd/ctrl/l2tp/l2tp.c @@ -293,10 +293,22 @@ static void l2tp_tunnel_free_session(void *sess) __l2tp_tunnel_free_session(sess); } +static void l2tp_tunnel_free_sessionid(void *data) +{ + uint16_t sid = (intptr_t)data; + struct l2tp_conn_t *conn = l2tp_tunnel_self(); + struct l2tp_sess_t *sess = l2tp_tunnel_get_session(conn, sid); + + if (sess) + l2tp_tunnel_free_session(sess); +} + static void l2tp_session_free(struct l2tp_sess_t *sess) { - triton_context_call(&sess->paren_conn->ctx, l2tp_tunnel_free_session, - sess); + intptr_t sid = sess->sid; + + triton_context_call(&sess->paren_conn->ctx, + l2tp_tunnel_free_sessionid, (void *)sid); } static void l2tp_tunnel_free(struct l2tp_conn_t *conn) -- cgit v1.2.3