@@ -1,6 +1,9 @@
/*
* wpa_supplicant/hostapd control interface library
* Copyright (c) 2004-2007, Jouni Malinen <j@w1.fi>
+ * Copyright (c) 2013, Quentin Casasnovas <quentinx.casasnovas@intel.com>
+ * Jean-Michel Bachot <jean-michel.bachot@intel.com>
+ * Loic Poulain <loicx.poulain@intel.com>
*
* This software may be distributed under the terms of the BSD license.
* See README for more details.
@@ -25,6 +28,11 @@
#include "private/android_filesystem_config.h"
#endif /* ANDROID */
+#ifdef CONFIG_CTRL_REF_COUNT
+#include <pthread.h>
+#include "utils/list.h"
+#endif /* CONFIG_CTRL_REF_COUNT */
+
#include "wpa_ctrl.h"
#include "common.h"
@@ -62,6 +70,103 @@ struct wpa_ctrl {
#endif /* CONFIG_CTRL_IFACE_NAMED_PIPE */
};
+#ifdef CONFIG_CTRL_REF_COUNT
+/**
+ * struct wpa_ctrl_meta - Internal meta data associated with a wpa_ctrl
+ */
+struct wpa_ctrl_meta {
+ struct wpa_ctrl *ctrl;
+ struct dl_list list;
+ int ref; /* reference counter on wpa_ctrl */
+};
+
+/* Internal protected linked list of wpa_ctrl_meta elements */
+static struct dl_list wpa_ctrl_list;
+static pthread_mutex_t list_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/* Return the meta data associated with a wpa_ctrl, or NULL if unavailable */
+static struct wpa_ctrl_meta * ctrl_to_meta(struct wpa_ctrl *ctrl)
+{
+ struct wpa_ctrl_meta *meta;
+
+ dl_list_for_each(meta, &wpa_ctrl_list, struct wpa_ctrl_meta, list) {
+ if (meta->ctrl == ctrl)
+ return meta;
+ }
+
+ return NULL;
+}
+
+/* Initialize the meta data of the wpa_ctrl and take reference */
+static int wpa_ctrl_init_meta(struct wpa_ctrl *ctrl)
+{
+ struct wpa_ctrl_meta *meta = os_malloc(sizeof(struct wpa_ctrl_meta));
+
+ if (!meta)
+ return -ENOMEM;
+
+ pthread_mutex_lock(&list_mutex);
+
+ if (!wpa_ctrl_list.next)
+ dl_list_init(&wpa_ctrl_list);
+
+ meta->ctrl = ctrl;
+ meta->ref = 1;
+
+ dl_list_add(&wpa_ctrl_list, &meta->list);
+
+ pthread_mutex_unlock(&list_mutex);
+
+ return 0;
+}
+
+/* Increment the wpa_ctrl reference counter, return 0 on success, -1 on error */
+static int wpa_ctrl_get(struct wpa_ctrl *ctrl)
+{
+ struct wpa_ctrl_meta *meta;
+ int res = 0;
+
+ pthread_mutex_lock(&list_mutex);
+ meta = ctrl_to_meta(ctrl);
+ if (meta)
+ meta->ref++;
+ else
+ res = -1;
+ pthread_mutex_unlock(&list_mutex);
+
+ return res;
+}
+
+/* Release the wpa_ctrl resources */
+static void wpa_ctrl_release(struct wpa_ctrl *ctrl);
+
+/* Decrement the wpa_ctrl reference counter, return 0 on success, -1 on error */
+static int wpa_ctrl_put(struct wpa_ctrl *ctrl)
+{
+ struct wpa_ctrl_meta *meta;
+ int res = 0;
+
+ pthread_mutex_lock(&list_mutex);
+ meta = ctrl_to_meta(ctrl);
+ if (meta) {
+ meta->ref--;
+ if (!meta->ref) {
+ dl_list_del(&meta->list);
+ os_free(meta);
+ wpa_ctrl_release(ctrl);
+ }
+ } else {
+ res = -1;
+ }
+ pthread_mutex_unlock(&list_mutex);
+
+ return res;
+}
+#else /* CONFIG_CTRL_REF_COUNT */
+static int wpa_ctrl_init_meta(struct wpa_ctrl *ctrl) {return 0;}
+static int wpa_ctrl_get(struct wpa_ctrl *ctrl) {return 0;}
+static int wpa_ctrl_put(struct wpa_ctrl *ctrl) {return 0;}
+#endif /* CONFIG_CTRL_REF_COUNT */
#ifdef CONFIG_CTRL_IFACE_UNIX
@@ -126,6 +231,13 @@ try_again:
return NULL;
}
+ if (wpa_ctrl_init_meta(ctrl)) {
+ close(ctrl->s);
+ unlink(ctrl->local.sun_path);
+ os_free(ctrl);
+ return NULL;
+ }
+
#ifdef ANDROID
chmod(ctrl->local.sun_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
chown(ctrl->local.sun_path, AID_SYSTEM, AID_WIFI);
@@ -206,6 +318,10 @@ try_again:
void wpa_ctrl_close(struct wpa_ctrl *ctrl)
{
+ wpa_ctrl_put(ctrl);
+#ifdef CONFIG_CTRL_REF_COUNT
+} void wpa_ctrl_release(struct wpa_ctrl *ctrl) {
+#endif /* CONFIG_CTRL_REF_COUNT */
if (ctrl == NULL)
return;
unlink(ctrl->local.sun_path);
@@ -343,6 +459,13 @@ struct wpa_ctrl * wpa_ctrl_open(const char *ctrl_path)
ctrl->remote_ip = os_strdup("localhost");
#endif /* CONFIG_CTRL_IFACE_UDP_REMOTE */
+ if (wpa_ctrl_init_meta(ctrl)) {
+ close(ctrl->s);
+ os_free(ctrl->remote_ip);
+ os_free(ctrl);
+ return NULL;
+ }
+
if (connect(ctrl->s, (struct sockaddr *) &ctrl->dest,
sizeof(ctrl->dest)) < 0) {
perror("connect");
@@ -371,14 +494,25 @@ char * wpa_ctrl_get_remote_ifname(struct wpa_ctrl *ctrl)
{
#define WPA_CTRL_MAX_PS_NAME 100
static char ps[WPA_CTRL_MAX_PS_NAME] = {};
+
+ if (wpa_ctrl_get(ctrl))
+ return NULL;
+
os_snprintf(ps, WPA_CTRL_MAX_PS_NAME, "%s/%s",
ctrl->remote_ip, ctrl->remote_ifname);
+
+ wpa_ctrl_put(ctrl);
+
return ps;
}
void wpa_ctrl_close(struct wpa_ctrl *ctrl)
{
+ wpa_ctrl_put(ctrl);
+#ifdef CONFIG_CTRL_REF_COUNT
+} void wpa_ctrl_release(struct wpa_ctrl *ctrl) {
+#endif /* CONFIG_CTRL_REF_COUNT */
close(ctrl->s);
os_free(ctrl->cookie);
os_free(ctrl->remote_ifname);
@@ -402,13 +536,16 @@ int wpa_ctrl_request(struct wpa_ctrl *ctrl, const char *cmd, size_t cmd_len,
char *cmd_buf = NULL;
size_t _cmd_len;
+ if (wpa_ctrl_get(ctrl))
+ return -EEXIST;
+
#ifdef CONFIG_CTRL_IFACE_UDP
if (ctrl->cookie) {
char *pos;
_cmd_len = os_strlen(ctrl->cookie) + 1 + cmd_len;
cmd_buf = os_malloc(_cmd_len);
if (cmd_buf == NULL)
- return -1;
+ goto error;
_cmd = cmd_buf;
pos = cmd_buf;
os_strlcpy(pos, ctrl->cookie, _cmd_len);
@@ -447,7 +584,7 @@ retry_send:
}
send_err:
os_free(cmd_buf);
- return -1;
+ goto error;
}
os_free(cmd_buf);
@@ -458,11 +595,11 @@ retry_send:
FD_SET(ctrl->s, &rfds);
res = select(ctrl->s + 1, &rfds, NULL, NULL, &tv);
if (res < 0)
- return res;
+ goto exit;
if (FD_ISSET(ctrl->s, &rfds)) {
res = recv(ctrl->s, reply, *reply_len, 0);
if (res < 0)
- return res;
+ goto exit;
if (res > 0 && reply[0] == '<') {
/* This is an unsolicited message from
* wpa_supplicant, not the reply to the
@@ -481,10 +618,24 @@ retry_send:
*reply_len = res;
break;
} else {
- return -2;
+ goto timeout;
}
}
- return 0;
+
+ res = 0;
+ goto exit;
+
+error:
+ res = -1;
+ goto exit;
+
+timeout:
+ res = -2;
+
+exit:
+ wpa_ctrl_put(ctrl);
+
+ return res;
}
#endif /* CTRL_IFACE_SOCKET */
@@ -522,11 +673,38 @@ int wpa_ctrl_detach(struct wpa_ctrl *ctrl)
int wpa_ctrl_recv(struct wpa_ctrl *ctrl, char *reply, size_t *reply_len)
{
int res;
+ fd_set rfds;
+ struct timeval tv;
+
+
+ for (;;) {
+ if (wpa_ctrl_get(ctrl))
+ return -EEXIST;
+
+ tv.tv_sec = 10;
+ tv.tv_usec = 0;
+
+ FD_ZERO(&rfds);
+ FD_SET(ctrl->s, &rfds);
+
+ if ((res = select(ctrl->s + 1, &rfds, NULL, NULL, &tv)) == -1) {
+ wpa_ctrl_put(ctrl);
+ break;
+ }
+
+ if (FD_ISSET(ctrl->s, &rfds)) {
+ res = recv(ctrl->s, reply, *reply_len, 0);
+ wpa_ctrl_put(ctrl);
+ break;
+ }
+
+ wpa_ctrl_put(ctrl);
+ }
- res = recv(ctrl->s, reply, *reply_len, 0);
if (res < 0)
return res;
*reply_len = res;
+
return 0;
}
@@ -534,19 +712,38 @@ int wpa_ctrl_recv(struct wpa_ctrl *ctrl, char *reply, size_t *reply_len)
int wpa_ctrl_pending(struct wpa_ctrl *ctrl)
{
struct timeval tv;
+ int res;
+
fd_set rfds;
tv.tv_sec = 0;
tv.tv_usec = 0;
FD_ZERO(&rfds);
+
+ if (wpa_ctrl_get(ctrl))
+ return -EEXIST;
+
FD_SET(ctrl->s, &rfds);
select(ctrl->s + 1, &rfds, NULL, NULL, &tv);
- return FD_ISSET(ctrl->s, &rfds);
+ res = FD_ISSET(ctrl->s, &rfds);
+
+ wpa_ctrl_put(ctrl);
+
+ return res;
}
int wpa_ctrl_get_fd(struct wpa_ctrl *ctrl)
{
- return ctrl->s;
+ int fd;
+
+ if (wpa_ctrl_get(ctrl))
+ return -EEXIST;
+
+ fd = ctrl->s;
+
+ wpa_ctrl_put(ctrl);
+
+ return fd;
}
#endif /* CTRL_IFACE_SOCKET */
Hello Jouni, list In response to thread "lib_wpa_client: making it thread safe?" We would like to propose the following patch which adds a reference counter on wpa_ctrl. We are going to integrate this patch in the Intel Android tree. The goal of this patch is to prevent a whole class crashes caused by freeing the wpa_ctrl while it's still in use by another thread. e.g in Android, this is typically the case, many crashes are reported due to libhardware_legacy (wifi) usage of this library. We had the following constraints in mind when writing the patch: - This patch should not block any thread. - This patch should not modify the lib API. - This patch should not modify the wpa_ctrl structure. - Modification are internal to the lib. - This feature can be enabled/disabled with a C flag: CONFIG_CTRL_REF_COUNT - This patch should fix the "issue" at lib level, avoiding implement workarounds at upper level General principle: - A wpa_ctrl resource cannot be freed if it still used by another thread. - A wpa_ctrl resource will be freed when closed and no longer used. - Prevent crashes If a lib function is used with an invalid/no longer valid wpa_ctrl identifier. - Main original code modifications consist in calling 'get' and 'put' on wpa_ctrl. We welcome any improvements you may have on this patch. BR, Loïc From: Loic Poulain <loicx.poulain@intel.com> This library provides a wpa_ctrl pointer to third parties. This pointer is used as a context identifier. But third parties can use this library asynchronously. So, if a third party closes a ctrl which is already used by an other request (ex: wpa_ctrl_request), then this context will no longer be valid, leading to a crash (NULL pointer dereference). In order to avoid such issue, this patch implements an internal reference counter so that a ctrl interface can be freed only if its reference counter reaches 0. Moreover, if a third party tries to call a library function with an invalid context/pointer, this function will handle this issue by returning an error. This patch is a first step to make this library thread safe. Signed-hostap: Loic Poulain <loicx.poulain@intel.com> Signed-hostap: Jean-Michel Bachot <jean-michel.bachot@intel.com> Signed-hostap: Quentin Casasnovas <quentinx.casasnovas@intel.com> --- src/common/wpa_ctrl.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 206 insertions(+), 9 deletions(-)