nl80211: improve error handling

 - introduce a new nl80211_request() which combines nl80211_msg() with
   nl80211_send() to simplify calling code

 - always invoke nl80211_free() in nl80211_send() and remove explicit
   nl80211_free() invocations in callers

 - back out early on netlink errors in functions performing multiple
   calls, e.g. when fetching scan results

Signed-off-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
index 3d9ee73681f0956e85351dd0edfc7298cac319b3..db0fc42efc2be19d4c0e3d0481a31b7bd83db0ee 100644 (file)
--- a/iwinfo_nl80211.c
+++ b/iwinfo_nl80211.c
@@ -374,7 +374,7 @@ static int nl80211_send(struct nl80211_msg_conveyor *cv,
                         void *cb_arg)
 {
        static struct nl80211_msg_conveyor rcv;
-       int err = 1;
+       int err;
 
        if (cb_func)
                nl_cb_set(cv->cb, NL_CB_VALID, NL_CB_CUSTOM, cb_func, cb_arg);
@@ -386,6 +386,8 @@ static int nl80211_send(struct nl80211_msg_conveyor *cv,
        if (err < 0)
                goto out;
 
+       err = 1;
+
        nl_cb_err(cv->cb,               NL_CB_CUSTOM, nl80211_msg_error,  &err);
        nl_cb_set(cv->cb, NL_CB_FINISH, NL_CB_CUSTOM, nl80211_msg_finish, &err);
        nl_cb_set(cv->cb, NL_CB_ACK,    NL_CB_CUSTOM, nl80211_msg_ack,    &err);
@@ -394,9 +396,24 @@ static int nl80211_send(struct nl80211_msg_conveyor *cv,
                nl_recvmsgs(nls->nl_sock, cv->cb);
 
 out:
+       nl80211_free(cv);
        return err;
 }
 
+static int nl80211_request(const char *ifname, int cmd, int flags,
+                           int (*cb_func)(struct nl_msg *, void *),
+                           void *cb_arg)
+{
+       struct nl80211_msg_conveyor *cv;
+
+       cv = nl80211_msg(ifname, cmd, flags);
+
+       if (!cv)
+               return -ENOMEM;
+
+       return nl80211_send(cv, cb_func, cb_arg);
+}
+
 static struct nlattr ** nl80211_parse(struct nl_msg *msg)
 {
        struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
@@ -443,18 +460,24 @@ static int nl80211_subscribe(const char *family, const char *group)
 {
        struct nl80211_group_conveyor cv = { .name = group, .id = -ENOENT };
        struct nl80211_msg_conveyor *req;
+       int err;
 
        req = nl80211_ctl(CTRL_CMD_GETFAMILY, 0);
        if (req)
        {
                NLA_PUT_STRING(req->msg, CTRL_ATTR_FAMILY_NAME, family);
-               nl80211_send(req, nl80211_subscribe_cb, &cv);
+               err = nl80211_send(req, nl80211_subscribe_cb, &cv);
+
+               if (err)
+                       return err;
+
+               return nl_socket_add_membership(nls->nl_sock, cv.id);
 
 nla_put_failure:
                nl80211_free(req);
        }
 
-       return nl_socket_add_membership(nls->nl_sock, cv.id);
+       return -ENOMEM;
 }
 
 
@@ -550,16 +573,11 @@ static int nl80211_ifname2phy_cb(struct nl_msg *msg, void *arg)
 static char * nl80211_ifname2phy(const char *ifname)
 {
        static char phy[32] = { 0 };
-       struct nl80211_msg_conveyor *req;
 
        memset(phy, 0, sizeof(phy));
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (req)
-       {
-               nl80211_send(req, nl80211_ifname2phy_cb, phy);
-               nl80211_free(req);
-       }
+       nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                       nl80211_ifname2phy_cb, phy);
 
        return phy[0] ? phy : NULL;
 }
@@ -642,17 +660,13 @@ static int nl80211_get_mode_cb(struct nl_msg *msg, void *arg)
 static int nl80211_get_mode(const char *ifname, int *buf)
 {
        char *res;
-       struct nl80211_msg_conveyor *req;
 
-       res = nl80211_phy2ifname(ifname);
-       req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0);
        *buf = IWINFO_OPMODE_UNKNOWN;
 
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_mode_cb, buf);
-               nl80211_free(req);
-       }
+       res = nl80211_phy2ifname(ifname);
+
+       nl80211_request(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0,
+                       nl80211_get_mode_cb, buf);
 
        return (*buf == IWINFO_OPMODE_UNKNOWN) ? -1 : 0;
 }
@@ -892,7 +906,7 @@ static int __nl80211_wpactl_query(const char *ifname, ...)
 
 static char * nl80211_ifadd(const char *ifname)
 {
-       char *rv = NULL, path[PATH_MAX];
+       char path[PATH_MAX];
        static char nif[IFNAMSIZ] = { 0 };
        struct nl80211_msg_conveyor *req;
        FILE *sysfs;
@@ -916,18 +930,19 @@ static char * nl80211_ifadd(const char *ifname)
                        fclose(sysfs);
                }
 
-               rv = nif;
+               return nif;
 
        nla_put_failure:
                nl80211_free(req);
        }
 
-       return rv;
+       return NULL;
 }
 
 static void nl80211_ifdel(const char *ifname)
 {
        struct nl80211_msg_conveyor *req;
+       int err;
 
        req = nl80211_msg(ifname, NL80211_CMD_DEL_INTERFACE, 0);
        if (req)
@@ -935,6 +950,7 @@ static void nl80211_ifdel(const char *ifname)
                NLA_PUT_STRING(req->msg, NL80211_ATTR_IFNAME, ifname);
 
                nl80211_send(req, NULL, NULL);
+               return;
 
        nla_put_failure:
                nl80211_free(req);
@@ -1035,47 +1051,33 @@ static int nl80211_get_ssid_bssid_cb(struct nl_msg *msg, void *arg)
 static int nl80211_get_ssid(const char *ifname, char *buf)
 {
        char *res;
-       struct nl80211_msg_conveyor *req;
-       struct nl80211_ssid_bssid sb;
+       struct nl80211_ssid_bssid sb = { .ssid = (unsigned char *)buf };
 
        /* try to find ssid from scan dump results */
        res = nl80211_phy2ifname(ifname);
-       req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
-
-       sb.ssid = (unsigned char *)buf;
-       *buf = 0;
+       sb.ssid[0] = 0;
 
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_ssid_bssid_cb, &sb);
-               nl80211_free(req);
-       }
+       nl80211_request(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP,
+                       nl80211_get_ssid_bssid_cb, &sb);
 
        /* failed, try to find from hostapd info */
-       if (*buf == 0)
-               nl80211_hostapd_query(ifname, "ssid", buf, IWINFO_ESSID_MAX_SIZE + 1);
+       if (sb.ssid[0] == 0)
+               nl80211_hostapd_query(ifname, "ssid", sb.ssid,
+                                     IWINFO_ESSID_MAX_SIZE + 1);
 
-       return (*buf == 0) ? -1 : 0;
+       return (sb.ssid[0] == 0) ? -1 : 0;
 }
 
 static int nl80211_get_bssid(const char *ifname, char *buf)
 {
        char *res, bssid[sizeof("FF:FF:FF:FF:FF:FF\0")];
-       struct nl80211_msg_conveyor *req;
-       struct nl80211_ssid_bssid sb;
+       struct nl80211_ssid_bssid sb = { };
 
        /* try to find bssid from scan dump results */
        res = nl80211_phy2ifname(ifname);
-       req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
 
-       sb.ssid = NULL;
-       sb.bssid[0] = 0;
-
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_ssid_bssid_cb, &sb);
-               nl80211_free(req);
-       }
+       nl80211_request(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP,
+                       nl80211_get_ssid_bssid_cb, &sb);
 
        /* failed, try to find mac from hostapd info */
        if ((sb.bssid[0] == 0) &&
@@ -1139,18 +1141,13 @@ static int nl80211_get_frequency_info_cb(struct nl_msg *msg, void *arg)
 static int nl80211_get_frequency(const char *ifname, int *buf)
 {
        char *res, channel[4], hwmode[2];
-       struct nl80211_msg_conveyor *req;
 
        /* try to find frequency from interface info */
        res = nl80211_phy2ifname(ifname);
-       req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0);
        *buf = 0;
 
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_frequency_info_cb, buf);
-               nl80211_free(req);
-       }
+       nl80211_request(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0,
+                       nl80211_get_frequency_info_cb, buf);
 
        /* failed, try to find frequency from hostapd info */
        if ((*buf == 0) &&
@@ -1164,13 +1161,9 @@ static int nl80211_get_frequency(const char *ifname, int *buf)
        if (*buf == 0)
        {
                res = nl80211_phy2ifname(ifname);
-               req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
 
-               if (req)
-               {
-                       nl80211_send(req, nl80211_get_frequency_scan_cb, buf);
-                       nl80211_free(req);
-               }
+               nl80211_request(res ? res : ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP,
+                               nl80211_get_frequency_scan_cb, buf);
        }
 
        return (*buf == 0) ? -1 : 0;
@@ -1201,21 +1194,15 @@ static int nl80211_get_txpower_cb(struct nl_msg *msg, void *arg)
 static int nl80211_get_txpower(const char *ifname, int *buf)
 {
        char *res;
-       struct nl80211_msg_conveyor *req;
 
        res = nl80211_phy2ifname(ifname);
-       req = nl80211_msg(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0);
+       *buf = 0;
 
-       if (req)
-       {
-               *buf = 0;
-               nl80211_send(req, nl80211_get_txpower_cb, buf);
-               nl80211_free(req);
-               if (*buf)
-                       return 0;
-       }
+       if (nl80211_request(res ? res : ifname, NL80211_CMD_GET_INTERFACE, 0,
+                           nl80211_get_txpower_cb, buf))
+               return -1;
 
-       return -1;
+       return 0;
 }
 
 
@@ -1283,7 +1270,6 @@ static void nl80211_fill_signal(const char *ifname, struct nl80211_rssi_rate *r)
 {
        DIR *d;
        struct dirent *de;
-       struct nl80211_msg_conveyor *req;
 
        r->rssi = 0;
        r->rate = 0;
@@ -1296,14 +1282,8 @@ static void nl80211_fill_signal(const char *ifname, struct nl80211_rssi_rate *r)
                            (!de->d_name[strlen(ifname)] ||
                             !strncmp(&de->d_name[strlen(ifname)], ".sta", 4)))
                        {
-                               req = nl80211_msg(de->d_name, NL80211_CMD_GET_STATION,
-                                                 NLM_F_DUMP);
-
-                               if (req)
-                               {
-                                       nl80211_send(req, nl80211_fill_signal_cb, r);
-                                       nl80211_free(req);
-                               }
+                               nl80211_request(de->d_name, NL80211_CMD_GET_STATION,
+                                               NLM_F_DUMP, nl80211_fill_signal_cb, r);
                        }
                }
 
@@ -1371,24 +1351,17 @@ static int nl80211_get_noise_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_noise(const char *ifname, int *buf)
 {
-       int8_t noise;
-       struct nl80211_msg_conveyor *req;
+       int8_t noise = 0;
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_SURVEY, NLM_F_DUMP);
-       if (req)
-       {
-               noise = 0;
-
-               nl80211_send(req, nl80211_get_noise_cb, &noise);
-               nl80211_free(req);
+       if (nl80211_request(ifname, NL80211_CMD_GET_SURVEY, NLM_F_DUMP,
+                           nl80211_get_noise_cb, &noise))
+               goto out;
 
-               if (noise)
-               {
-                       *buf = noise;
-                       return 0;
-               }
-       }
+       *buf = noise;
+       return 0;
 
+out:
+       *buf = 0;
        return -1;
 }
 
@@ -1792,7 +1765,6 @@ static int nl80211_get_assoclist(const char *ifname, char *buf, int *len)
        DIR *d;
        int i, noise = 0;
        struct dirent *de;
-       struct nl80211_msg_conveyor *req;
        struct nl80211_array_buf arr = { .buf = buf, .count = 0 };
        struct iwinfo_assoclist_entry *e;
 
@@ -1804,14 +1776,8 @@ static int nl80211_get_assoclist(const char *ifname, char *buf, int *len)
                            (!de->d_name[strlen(ifname)] ||
                             !strncmp(&de->d_name[strlen(ifname)], ".sta", 4)))
                        {
-                               req = nl80211_msg(de->d_name, NL80211_CMD_GET_STATION,
-                                                 NLM_F_DUMP);
-
-                               if (req)
-                               {
-                                       nl80211_send(req, nl80211_get_assoclist_cb, &arr);
-                                       nl80211_free(req);
-                               }
+                               nl80211_request(de->d_name, NL80211_CMD_GET_STATION,
+                                               NLM_F_DUMP, nl80211_get_assoclist_cb, &arr);
                        }
                }
 
@@ -1879,7 +1845,7 @@ static int nl80211_get_txpwrlist_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_txpwrlist(const char *ifname, char *buf, int *len)
 {
-       int ch_cur;
+       int err, ch_cur;
        int dbm_max = -1, dbm_cur, dbm_cnt;
        struct nl80211_msg_conveyor *req;
        struct iwinfo_txpwrlist_entry entry;
@@ -1887,17 +1853,13 @@ static int nl80211_get_txpwrlist(const char *ifname, char *buf, int *len)
        if (nl80211_get_channel(ifname, &ch_cur))
                ch_cur = 0;
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (req)
-       {
-               /* initialize the value pointer with channel for callback */
-               dbm_max = ch_cur;
+       /* initialize the value pointer with channel for callback */
+       dbm_max = ch_cur;
 
-               nl80211_send(req, nl80211_get_txpwrlist_cb, &dbm_max);
-               nl80211_free(req);
-       }
+       err = nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                             nl80211_get_txpwrlist_cb, &dbm_max);
 
-       if (dbm_max > 0)
+       if (!err)
        {
                for (dbm_cur = 0, dbm_cnt = 0;
                     dbm_cur < dbm_max;
@@ -2100,27 +2062,24 @@ static int nl80211_get_scanlist_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_scanlist_nl(const char *ifname, char *buf, int *len)
 {
-       struct nl80211_msg_conveyor *req;
        struct nl80211_scanlist sl = { .e = (struct iwinfo_scanlist_entry *)buf };
 
-       req = nl80211_msg(ifname, NL80211_CMD_TRIGGER_SCAN, 0);
-       if (req)
-       {
-               nl80211_send(req, NULL, NULL);
-               nl80211_free(req);
-       }
+       if (nl80211_request(ifname, NL80211_CMD_TRIGGER_SCAN, 0, NULL, NULL))
+               goto out;
 
-       nl80211_wait("nl80211", "scan", NL80211_CMD_NEW_SCAN_RESULTS);
+       if (nl80211_wait("nl80211", "scan", NL80211_CMD_NEW_SCAN_RESULTS))
+               goto out;
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_scanlist_cb, &sl);
-               nl80211_free(req);
-       }
+       if (nl80211_request(ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP,
+                           nl80211_get_scanlist_cb, &sl))
+               goto out;
 
        *len = sl.len * sizeof(struct iwinfo_scanlist_entry);
-       return *len ? 0 : -1;
+       return 0;
+
+out:
+       *len = 0;
+       return -1;
 }
 
 static int wpasupp_ssid_decode(const char *in, char *out, int outlen)
@@ -2447,22 +2406,17 @@ static int nl80211_get_freqlist_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_freqlist(const char *ifname, char *buf, int *len)
 {
-       struct nl80211_msg_conveyor *req;
        struct nl80211_array_buf arr = { .buf = buf, .count = 0 };
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_freqlist_cb, &arr);
-               nl80211_free(req);
-       }
+       if (nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                           nl80211_get_freqlist_cb, &arr))
+               goto out;
 
-       if (arr.count > 0)
-       {
-               *len = arr.count * sizeof(struct iwinfo_freqlist_entry);
-               return 0;
-       }
+       *len = arr.count * sizeof(struct iwinfo_freqlist_entry);
+       return 0;
 
+out:
+       *len = 0;
        return -1;
 }
 
@@ -2481,20 +2435,11 @@ static int nl80211_get_country_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_country(const char *ifname, char *buf)
 {
-       int rv = -1;
-       struct nl80211_msg_conveyor *req;
-
-       req = nl80211_msg(ifname, NL80211_CMD_GET_REG, 0);
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_country_cb, buf);
-               nl80211_free(req);
-
-               if (buf[0])
-                       rv = 0;
-       }
+       if (nl80211_request(ifname, NL80211_CMD_GET_REG, 0,
+                           nl80211_get_country_cb, buf))
+               return -1;
 
-       return rv;
+       return 0;
 }
 
 static int nl80211_get_countrylist(const char *ifname, char *buf, int *len)
@@ -2604,43 +2549,39 @@ static int nl80211_get_modelist_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_hwmodelist(const char *ifname, int *buf)
 {
-       struct nl80211_msg_conveyor *req;
        struct nl80211_modes m = { 0 };
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_modelist_cb, &m);
-               nl80211_free(req);
-       }
+       if (nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                           nl80211_get_modelist_cb, &m))
+               goto out;
 
-       if (m.ok)
-       {
-               *buf = m.hw;
-               return 0;
-       }
+       if (!m.ok)
+               goto out;
+
+       *buf = m.hw;
+       return 0;
 
+out:
+       *buf = 0;
        return -1;
 }
 
 static int nl80211_get_htmodelist(const char *ifname, int *buf)
 {
-       struct nl80211_msg_conveyor *req;
        struct nl80211_modes m = { 0 };
 
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (req)
-       {
-               nl80211_send(req, nl80211_get_modelist_cb, &m);
-               nl80211_free(req);
-       }
+       if (nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                           nl80211_get_modelist_cb, &m))
+               goto out;
 
-       if (m.ok)
-       {
-               *buf = m.ht;
-               return 0;
-       }
+       if (!m.ok)
+               goto out;
 
+       *buf = m.ht;
+       return 0;
+
+out:
+       *buf = 0;
        return -1;
 }
 
@@ -2700,14 +2641,10 @@ static int nl80211_get_ifcomb_cb(struct nl_msg *msg, void *arg)
 
 static int nl80211_get_mbssid_support(const char *ifname, int *buf)
 {
-       struct nl80211_msg_conveyor *req;
-
-       req = nl80211_msg(ifname, NL80211_CMD_GET_WIPHY, 0);
-       if (!req)
+       if (nl80211_request(ifname, NL80211_CMD_GET_WIPHY, 0,
+                           nl80211_get_ifcomb_cb, buf))
                return -1;
 
-       nl80211_send(req, nl80211_get_ifcomb_cb, buf);
-       nl80211_free(req);
        return 0;
 }