handshake: Allow event handler to free handshake

Like in ap.c, allow the event callback to mark the handshake state as
destroyed, without causing invalid accesses after the callback has
returned.  In this case the crash was because try_handshake_complete
needed to access members of handshake_state after emitting the event,
as well as access the netdev, which also has been destroyed:

==257707== Invalid read of size 8
==257707==    at 0x408C85: try_handshake_complete (netdev.c:1487)
==257707==    by 0x408C85: try_handshake_complete (netdev.c:1480)
(...)
==257707==  Address 0x4e187e8 is 856 bytes inside a block of size 872 free'd
==257707==    at 0x484621F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==257707==    by 0x437887: ap_stop_handshake (ap.c:151)
==257707==    by 0x439793: ap_del_station (ap.c:316)
==257707==    by 0x43EA92: ap_station_disconnect (ap.c:3411)
==257707==    by 0x43EA92: ap_station_disconnect (ap.c:3399)
==257707==    by 0x454276: p2p_group_event (p2p.c:1006)
==257707==    by 0x439147: ap_event (ap.c:281)
==257707==    by 0x4393AB: ap_new_rsna (ap.c:390)
==257707==    by 0x4393AB: ap_handshake_event (ap.c:1010)
==257707==    by 0x408C7F: try_handshake_complete (netdev.c:1485)
==257707==    by 0x408C7F: try_handshake_complete (netdev.c:1480)
(...)

src/handshake.c

@@ -107,6 +107,11 @@ void handshake_state_free(struct handshake_state *s)
 {
 	__typeof__(s->free) destroy = s->free;
 
+	if (s->in_event) {
+		s->in_event = false;
+		return;
+	}
+
 	l_free(s->authenticator_ie);
 	l_free(s->supplicant_ie);
 	l_free(s->authenticator_rsnxe);

src/handshake.h

@@ -159,17 +159,29 @@ struct handshake_state {
 	void *user_data;
 
 	void (*free)(struct handshake_state *s);
+	bool in_event;
 
 	handshake_event_func_t event_func;
 };
 
-#define handshake_event(hs, event, ...)	\
-	do {	\
-		if (!(hs)->event_func)	\
-			break;	\
+#define handshake_event(_hs, event, ...)	\
+	(__extension__ ({	\
+		struct handshake_state *hs = (_hs);	\
+		bool freed = false;	\
 	\
-		(hs)->event_func((hs), event, (hs)->user_data, ##__VA_ARGS__); \
-	} while (0)
+		if (hs->event_func && !hs->in_event) {	\
+			hs->in_event = true;	\
+			hs->event_func(hs, event, hs->user_data,	\
+					##__VA_ARGS__); \
+	\
+			if (!hs->in_event) {	\
+				handshake_state_free(hs);	\
+				freed = true;	\
+			} else	\
+				hs->in_event = false;	\
+		}	\
+		freed;	\
+	}))
 
 void handshake_state_free(struct handshake_state *s);
 

src/netdev.c

@@ -1482,7 +1482,9 @@ static void try_handshake_complete(struct netdev_handshake_state *nhs)
 	if (nhs->ptk_installed && nhs->gtk_installed && nhs->igtk_installed &&
 			!nhs->complete) {
 		nhs->complete = true;
-		handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE);
+
+		if (handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE))
+			return;
 
 		if (nhs->netdev->type == NL80211_IFTYPE_STATION ||
 				nhs->netdev->type == NL80211_IFTYPE_P2P_CLIENT)
@@ -2006,7 +2008,9 @@ static void netdev_group_timeout_cb(struct l_timeout *timeout, void *user_data)
 			nhs->netdev->index);
 
 	nhs->complete = true;
-	handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE);
+
+	if (handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE))
+		return;
 
 	netdev_connect_ok(nhs->netdev);
 }
@@ -2155,7 +2159,9 @@ static void netdev_set_pmk_cb(struct l_genl_msg *msg, void *user_data)
 		return;
 	}
 
-	handshake_event(netdev->handshake, HANDSHAKE_EVENT_SETTING_KEYS);
+	if (handshake_event(netdev->handshake, HANDSHAKE_EVENT_SETTING_KEYS))
+		return;
+
 	netdev_connect_ok(netdev);
 }
 
@@ -2906,8 +2912,8 @@ static void netdev_connect_event(struct l_genl_msg *msg, struct netdev *netdev)
 	}
 
 	/* Allow station to sync the PSK to disk */
-	if (is_offload(hs))
-		handshake_event(hs, HANDSHAKE_EVENT_SETTING_KEYS);
+	if (is_offload(hs) && handshake_event(hs, HANDSHAKE_EVENT_SETTING_KEYS))
+		return;
 
 	netdev_connect_ok(netdev);
 	return;