improve connect vs. notification race behaviour

This doesn't fully fix it, but should fix it
in the case there's only one application (iw)
trying to connect.

diff --git a/connect.c b/connect.c
index ced289fad961393717966d4a8bad8ea3b063751b..1af4e0dcd65beab1b33d38603ec607c497b85c89 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -95,6 +95,11 @@ static int iw_connect(struct nl80211_state *state, struct nl_cb *cb,
        conn_argv = calloc(conn_argc, sizeof(*conn_argv));
        if (!conn_argv)
                return -ENOMEM;
+
+       err = __prepare_listen_events(state);
+       if (err)
+               return err;
+
        conn_argv[0] = dev;
        conn_argv[1] = "connect";
        conn_argv[2] = "establish";
@@ -111,26 +116,25 @@ static int iw_connect(struct nl80211_state *state, struct nl_cb *cb,
        /*
         * WARNING: DO NOT COPY THIS CODE INTO YOUR APPLICATION
         *
-        * This code has a bug, which requires creating a separate
-        * nl80211 socket to fix:
-        * It is possible for a NL80211_CMD_NEW_SCAN_RESULTS or
-        * NL80211_CMD_SCAN_ABORTED message to be sent by the kernel
-        * before (!) we listen to it, because we only start listening
-        * after we send our scan request.
+        * This code has a bug:
         *
-        * Doing it the other way around has a race condition as well,
-        * if you first open the events socket you may get a notification
-        * for a previous scan.
+        * It is possible for a connect result message from another
+        * connect attempt to be processed here first, because we
+        * start listening to the multicast group before starting
+        * our own connect request, which may succeed but we get a
+        * fail message from a previous attempt that raced with us,
+        * or similar.
         *
         * The only proper way to fix this would be to listen to events
         * before sending the command, and for the kernel to send the
-        * connect request along with the event, so that you can match
-        * up whether the connect _you_ requested was finished or aborted.
+        * connect request or a cookie along with the event, so that you
+        * can match up whether the connect _you_ requested was finished
+        * or aborted.
         *
         * Alas, the kernel doesn't do that (yet).
         */
 
-       __listen_events(state, ARRAY_SIZE(cmds), cmds, &printargs);
+       __do_listen_events(state, ARRAY_SIZE(cmds), cmds, &printargs);
        return 0;
 }
 TOPLEVEL(connect, "[-w] <SSID> [<freq in MHz>] [<bssid>] [key 0:abcde d:1:6162636465]",

diff --git a/event.c b/event.c
index 2f61f511d1c3ed1ba73fef22cae73639dfd8600e..129db1a9e38038426c61c0302bf6fd80d689eb42 100644 (file)
--- a/event.c
+++ b/event.c
@@ -377,7 +377,7 @@ static int wait_event(struct nl_msg *msg, void *arg)
        return NL_SKIP;
 }
 
-static int __prepare_listen_events(struct nl80211_state *state)
+int __prepare_listen_events(struct nl80211_state *state)
 {
        int mcid, ret;
 
@@ -417,23 +417,18 @@ static int __prepare_listen_events(struct nl80211_state *state)
        return 0;
 }
 
-__u32 __listen_events(struct nl80211_state *state,
-                     const int n_waits, const __u32 *waits,
-                     struct print_event_args *args)
+__u32 __do_listen_events(struct nl80211_state *state,
+                        const int n_waits, const __u32 *waits,
+                        struct print_event_args *args)
 {
        struct nl_cb *cb = nl_cb_alloc(iw_debug ? NL_CB_DEBUG : NL_CB_DEFAULT);
        struct wait_event wait_ev;
-       int ret;
 
        if (!cb) {
                fprintf(stderr, "failed to allocate netlink callbacks\n");
                return -ENOMEM;
        }
 
-       ret = __prepare_listen_events(state);
-       if (ret)
-               return ret;
-
        /* no sequence checking for multicast messages */
        nl_cb_set(cb, NL_CB_SEQ_CHECK, NL_CB_CUSTOM, no_seq_check, NULL);
 
@@ -458,7 +453,13 @@ __u32 __listen_events(struct nl80211_state *state,
 __u32 listen_events(struct nl80211_state *state,
                    const int n_waits, const __u32 *waits)
 {
-       return __listen_events(state, n_waits, waits, NULL);
+       int ret;
+
+       ret = __prepare_listen_events(state);
+       if (ret)
+               return ret;
+
+       return __do_listen_events(state, n_waits, waits, NULL);
 }
 
 static int print_events(struct nl80211_state *state,
@@ -467,6 +468,7 @@ static int print_events(struct nl80211_state *state,
                        int argc, char **argv)
 {
        struct print_event_args args;
+       int ret;
 
        memset(&args, 0, sizeof(args));
 
@@ -487,7 +489,11 @@ static int print_events(struct nl80211_state *state,
        if (argc)
                return 1;
 
-       return __listen_events(state, 0, NULL, &args);
+       ret = __prepare_listen_events(state);
+       if (ret)
+               return ret;
+
+       return __do_listen_events(state, 0, NULL, &args);
 }
 TOPLEVEL(event, "[-t] [-f]", 0, 0, CIB_NONE, print_events,
        "Monitor events from the kernel.\n"

diff --git a/iw.h b/iw.h
index d1608e8d71e4cae09442cc31f58c6231d4d13ca9..c21f093d4e93833e2b83e0fe898c81ba7ebb5c45 100644 (file)
--- a/iw.h
+++ b/iw.h
@@ -110,9 +110,10 @@ struct print_event_args {
 
 __u32 listen_events(struct nl80211_state *state,
                    const int n_waits, const __u32 *waits);
-__u32 __listen_events(struct nl80211_state *state,
-                     const int n_waits, const __u32 *waits,
-                     struct print_event_args *args);
+int __prepare_listen_events(struct nl80211_state *state);
+__u32 __do_listen_events(struct nl80211_state *state,
+                        const int n_waits, const __u32 *waits,
+                        struct print_event_args *args);
 
 
 int mac_addr_a2n(unsigned char *mac_addr, char *arg);