Fix use-after-free in wifi_cleanup()
am: 2ce1c4952d
* commit '2ce1c4952d871f2baad0bc69dd8500f830f43db4':
Fix use-after-free in wifi_cleanup()
diff --git a/bcmdhd/firmware/bcm4358/fw_bcm4358.bin b/bcmdhd/firmware/bcm4358/fw_bcm4358.bin
index 56cb61f..70ea345 100644
--- a/bcmdhd/firmware/bcm4358/fw_bcm4358.bin
+++ b/bcmdhd/firmware/bcm4358/fw_bcm4358.bin
Binary files differ
diff --git a/bcmdhd/firmware/bcm4358/fw_bcm4358_ap.bin b/bcmdhd/firmware/bcm4358/fw_bcm4358_ap.bin
index ab80660..50b4fbe 100644
--- a/bcmdhd/firmware/bcm4358/fw_bcm4358_ap.bin
+++ b/bcmdhd/firmware/bcm4358/fw_bcm4358_ap.bin
Binary files differ
diff --git a/bcmdhd/wifi_hal/common.h b/bcmdhd/wifi_hal/common.h
index 8c09c16..118e0fc 100644
--- a/bcmdhd/wifi_hal/common.h
+++ b/bcmdhd/wifi_hal/common.h
@@ -154,7 +154,7 @@
int cleanup_socks[2]; // sockets used to implement wifi_cleanup
bool in_event_loop; // Indicates that event loop is active
- bool clean_up; // Indication to clean up the socket
+ bool clean_up; // Indication to exit since cleanup has started
wifi_internal_event_handler event_handler; // default event handler
wifi_cleaned_up_handler cleaned_up_handler; // socket cleaned up handler
diff --git a/bcmdhd/wifi_hal/gscan.cpp b/bcmdhd/wifi_hal/gscan.cpp
index 8a21a0d..418537b 100644
--- a/bcmdhd/wifi_hal/gscan.cpp
+++ b/bcmdhd/wifi_hal/gscan.cpp
@@ -19,7 +19,7 @@
#include "sync.h"
#define LOG_TAG "WifiHAL"
-//#define LOG_NDEBUG 1 //uncomment to enable verbose logging
+//#define LOG_NDEBUG 0 //uncomment to enable verbose logging
#include <utils/Log.h>
@@ -923,8 +923,8 @@
mCompleted = it.get_u8();
ALOGV("retrieved mCompleted flag : %d", mCompleted);
} else if (it.get_type() == GSCAN_ATTRIBUTE_SCAN_RESULTS || it.get_type() == 0) {
+ int scan_id = 0, flags = 0, num = 0;
for (nl_iterator it2(it.get()); it2.has_next(); it2.next()) {
- int scan_id = 0, flags = 0, num = 0;
if (it2.get_type() == GSCAN_ATTRIBUTE_SCAN_ID) {
scan_id = it2.get_u32();
ALOGV("retrieved scan_id : 0x%0x", scan_id);
@@ -935,6 +935,10 @@
num = it2.get_u32();
ALOGV("retrieved num_results: %d", num);
} else if (it2.get_type() == GSCAN_ATTRIBUTE_SCAN_RESULTS) {
+ if (mRetrieved >= mMax) {
+ ALOGW("Stored %d scans, ignoring excess results", mRetrieved);
+ break;
+ }
num = it2.get_len() / sizeof(wifi_scan_result);
num = min(MAX_RESULTS - mNextScanResult, num);
num = min((int)MAX_AP_CACHE_PER_SCAN, num);
@@ -952,13 +956,11 @@
mScans[mRetrieved].scan_id = scan_id;
mScans[mRetrieved].flags = flags;
mScans[mRetrieved].num_results = num;
+ ALOGV("Setting result of scan_id : 0x%0x", mScans[mRetrieved].scan_id);
memcpy(mScans[mRetrieved].results,
&(mScanResults[mNextScanResult]), num * sizeof(wifi_scan_result));
mNextScanResult += num;
mRetrieved++;
- if (mRetrieved >= mMax && it.has_next()) {
- ALOGW("Ignoring attributes after this scan");
- }
} else {
ALOGW("Ignoring invalid attribute type = %d, size = %d",
it.get_type(), it.get_len());
diff --git a/bcmdhd/wifi_hal/wifi_hal.cpp b/bcmdhd/wifi_hal/wifi_hal.cpp
index 982702a..fdb7d7b 100644
--- a/bcmdhd/wifi_hal/wifi_hal.cpp
+++ b/bcmdhd/wifi_hal/wifi_hal.cpp
@@ -179,12 +179,14 @@
ALOGI("Creating socket");
if (socketpair(AF_UNIX, SOCK_STREAM, 0, info->cleanup_socks) == -1) {
ALOGE("Could not create cleanup sockets");
+ free(info);
return WIFI_ERROR_UNKNOWN;
}
struct nl_sock *cmd_sock = wifi_create_nl_socket(WIFI_HAL_CMD_SOCK_PORT);
if (cmd_sock == NULL) {
ALOGE("Could not create handle");
+ free(info);
return WIFI_ERROR_UNKNOWN;
}
@@ -192,12 +194,16 @@
if (event_sock == NULL) {
ALOGE("Could not create handle");
nl_socket_free(cmd_sock);
+ free(info);
return WIFI_ERROR_UNKNOWN;
}
struct nl_cb *cb = nl_socket_get_cb(event_sock);
if (cb == NULL) {
ALOGE("Could not create handle");
+ nl_socket_free(cmd_sock);
+ nl_socket_free(event_sock);
+ free(info);
return WIFI_ERROR_UNKNOWN;
}
@@ -288,9 +294,28 @@
void wifi_cleanup(wifi_handle handle, wifi_cleaned_up_handler handler)
{
hal_info *info = getHalInfo(handle);
- info->cleaned_up_handler = handler;
- info->clean_up = true;
+ char buf[64];
+ info->cleaned_up_handler = handler;
+ if (write(info->cleanup_socks[0], "Exit", 4) < 1) {
+ // As a fallback set the cleanup flag to TRUE
+ ALOGE("could not write to the cleanup socket");
+ } else {
+ // Listen to the response
+ // Hopefully we dont get errors or get hung up
+ // Not much can be done in that case, but assume that
+ // it has rx'ed the Exit message to exit the thread.
+ // As a fallback set the cleanup flag to TRUE
+ memset(buf, 0, sizeof(buf));
+ int result = read(info->cleanup_socks[0], buf, sizeof(buf));
+ ALOGE("%s: Read after POLL returned %d, error no = %d", __FUNCTION__, result, errno);
+ if (strncmp(buf, "Done", 4) == 0) {
+ ALOGE("Event processing terminated");
+ } else {
+ ALOGD("Rx'ed %s", buf);
+ }
+ }
+ info->clean_up = true;
pthread_mutex_lock(&info->cb_lock);
int bad_commands = 0;
@@ -298,7 +323,7 @@
for (int i = 0; i < info->num_event_cb; i++) {
cb_info *cbi = &(info->event_cb[i]);
WifiCommand *cmd = (WifiCommand *)cbi->cb_arg;
- ALOGE("Command left in event_cb %p:%s", cmd, cmd->getType());
+ ALOGI("Command left in event_cb %p:%s", cmd, (cmd ? cmd->getType(): ""));
}
while (info->num_cmd > bad_commands) {
@@ -306,12 +331,12 @@
cmd_info *cmdi = &(info->cmd[bad_commands]);
WifiCommand *cmd = cmdi->cmd;
if (cmd != NULL) {
- ALOGD("Cancelling command %p:%s", cmd, cmd->getType());
+ ALOGI("Cancelling command %p:%s", cmd, cmd->getType());
pthread_mutex_unlock(&info->cb_lock);
cmd->cancel();
pthread_mutex_lock(&info->cb_lock);
if (num_cmd == info->num_cmd) {
- ALOGE("Cancelling command %p:%s did not work", cmd, cmd->getType());
+ ALOGI("Cancelling command %p:%s did not work", cmd, (cmd ? cmd->getType(): ""));
bad_commands++;
}
/* release reference added when command is saved */
@@ -324,14 +349,8 @@
WifiCommand *cmd = (WifiCommand *)cbi->cb_arg;
ALOGE("Leaked command %p", cmd);
}
-
pthread_mutex_unlock(&info->cb_lock);
-
- if (write(info->cleanup_socks[0], "T", 1) < 1) {
- ALOGE("could not write to cleanup socket");
- } else {
- ALOGI("Wifi cleanup completed");
- }
+ internal_cleaned_up_handler(handle);
}
static int internal_pollin_handler(wifi_handle handle)
@@ -384,16 +403,23 @@
// ALOGI("Found some events!!!");
internal_pollin_handler(handle);
} else if (pfd[1].revents & POLLIN) {
- ALOGI("Got a signal to exit!!!");
+ memset(buf, 0, sizeof(buf));
int result2 = read(pfd[1].fd, buf, sizeof(buf));
- ALOGE("Read after POLL returned %d, error no = %d", result2, errno);
+ ALOGE("%s: Read after POLL returned %d, error no = %d", __FUNCTION__, result2, errno);
+ if (strncmp(buf, "Exit", 4) == 0) {
+ ALOGD("Got a signal to exit!!!");
+ if (write(pfd[1].fd, "Done", 4) < 1) {
+ ALOGE("could not write to the cleanup socket");
+ }
+ break;
+ } else {
+ ALOGD("Rx'ed %s on the cleanup socket\n", buf);
+ }
} else {
ALOGE("Unknown event - %0x, %0x", pfd[0].revents, pfd[1].revents);
}
} while (!info->clean_up);
-
- ALOGI("Cleaning up");
- internal_cleaned_up_handler(handle);
+ ALOGI("Exit %s", __FUNCTION__);
}
///////////////////////////////////////////////////////////////////////////////////////
@@ -454,10 +480,9 @@
if (cmd != NULL) {
cmd->addRef();
}
-
pthread_mutex_unlock(&info->cb_lock);
-
- (*cb_func)(msg, cb_arg);
+ if (cb_func)
+ (*cb_func)(msg, cb_arg);
if (cmd != NULL) {
cmd->releaseRef();
}
@@ -1009,7 +1034,12 @@
wifi_handle handle = getWifiHandle(iface);
SetRSSIMonitorCommand *cmd = new SetRSSIMonitorCommand(id, iface, max_rssi, min_rssi, eh);
wifi_register_cmd(handle, id, cmd);
- return (wifi_error)cmd->start();
+
+ wifi_error result = (wifi_error)cmd->start();
+ if (result != WIFI_SUCCESS) {
+ wifi_unregister_cmd(handle, id);
+ }
+ return result;
}
