[PATCH] lib_wpa_client: Add a reference counter on wpa_ctrl.
Poulain, LoicX
loicx.poulain
Fri Jul 19 08:26:41 PDT 2013
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 at 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 at intel.com>
Signed-hostap: Jean-Michel Bachot <jean-michel.bachot at intel.com>
Signed-hostap: Quentin Casasnovas <quentinx.casasnovas at intel.com>
---
src/common/wpa_ctrl.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 206 insertions(+), 9 deletions(-)
diff --git a/src/common/wpa_ctrl.c b/src/common/wpa_ctrl.c
index d9a7509..fa7e823 100644
--- a/src/common/wpa_ctrl.c
+++ b/src/common/wpa_ctrl.c
@@ -1,6 +1,9 @@
/*
* wpa_supplicant/hostapd control interface library
* Copyright (c) 2004-2007, Jouni Malinen <j at w1.fi>
+ * Copyright (c) 2013, Quentin Casasnovas <quentinx.casasnovas at intel.com>
+ * Jean-Michel Bachot <jean-michel.bachot at intel.com>
+ * Loic Poulain <loicx.poulain at 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 */
--
1.7.0.4
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
More information about the Hostap
mailing list