This patch series contains changes to fix memory leaks, avoid NULL pointer
exceptions and checkpatch reported issue fixes.
Ajay Singh (9):
staging: wilc1000: remove unused global variables related to p2p
staging: wilc1000: avoid 'NULL' pointer access in
wilc_network_info_received()
staging: wilc1000: free allocated memory in edit and add station
functions
staging: wilc1000: free memory allocated in add wep key functions
staging: wilc1000: free allocated memory after processing
wilc_send_config_pkt()
staging: wilc1000: fix to free allocated memory in wilc_add_ptk()
staging: wilc1000: free allocated memory in wilc_add_rx_gtk()
staging: wilc1000: split handle_rcvd_gnrl_async_info() to avoid
leading tabs
staging: wilc1000: free memory allocated for general info message from
firmware
drivers/staging/wilc1000/host_interface.c | 421 +++++++++++++++---------------
1 file changed, 204 insertions(+), 217 deletions(-)
--
2.7.4
On Mon, 26 Mar 2018 11:32:41 +0300
Dan Carpenter <[email protected]> wrote:
> What happened to patch 8/9? Anyway, I can't apply this patch and it
> could be my fault or it could be the missing patch. I don't know...
I rechecked by applying the patches in order and didn't face any conflict.
I am going to send the v2 for this patch series by including the review
comments.
Regards,
Ajay
Free allocated memory after completing wilc_send_config_pkt() function.
Remove unncessary use of 'stamac' pointer in handle_get_inactive_time().
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index c958dd3..f493c78 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1909,7 +1909,6 @@ static s32 handle_get_inactive_time(struct wilc_vif *vif,
struct sta_inactive_t *hif_sta_inactive)
{
s32 result = 0;
- u8 *stamac;
struct wid wid;
struct host_if_drv *hif_drv = vif->hif_drv;
@@ -1920,11 +1919,11 @@ static s32 handle_get_inactive_time(struct wilc_vif *vif,
if (!wid.val)
return -ENOMEM;
- stamac = wid.val;
- ether_addr_copy(stamac, hif_sta_inactive->mac);
+ ether_addr_copy(wid.val, hif_sta_inactive->mac);
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
wilc_get_vif_idx(vif));
+ kfree(wid.val);
if (result) {
netdev_err(vif->ndev, "Failed to SET inactive time\n");
@@ -2225,6 +2224,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
wilc_get_vif_idx(vif));
+ kfree(wid.val);
if (result != 0)
netdev_err(vif->ndev, "Failed to set remain on channel\n");
@@ -2269,6 +2269,7 @@ static int handle_register_frame(struct wilc_vif *vif,
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
wilc_get_vif_idx(vif));
+ kfree(wid.val);
if (result) {
netdev_err(vif->ndev, "Failed to frame register\n");
result = -EINVAL;
@@ -2300,6 +2301,7 @@ static u32 handle_listen_state_expired(struct wilc_vif *vif,
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
wilc_get_vif_idx(vif));
+ kfree(wid.val);
if (result != 0) {
netdev_err(vif->ndev, "Failed to set remain channel\n");
goto _done_;
--
2.7.4
Cleanup patch to remove the unused global variables defined for p2p.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 59 -------------------------------
1 file changed, 59 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 5082ede..a13998d 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -254,13 +254,6 @@ static u32 inactive_time;
static u8 del_beacon;
static u32 clients_count;
-static u8 *join_req;
-static u8 *info_element;
-static u8 mode_11i;
-static u8 auth_type;
-static u32 join_req_size;
-static u32 info_element_size;
-static struct wilc_vif *join_req_vif;
#define REAL_JOIN_REQ 0
#define FLUSHED_JOIN_REQ 1
#define FLUSHED_BYTE_POS 79
@@ -995,39 +988,23 @@ static s32 handle_connect(struct wilc_vif *vif,
wid_list[wid_cnt].size = hif_drv->usr_conn_req.ies_len;
wid_cnt++;
- if (memcmp("DIRECT-", conn_attr->ssid, 7)) {
- info_element_size = hif_drv->usr_conn_req.ies_len;
- info_element = kmalloc(info_element_size, GFP_KERNEL);
- memcpy(info_element, hif_drv->usr_conn_req.ies,
- info_element_size);
- }
wid_list[wid_cnt].id = (u16)WID_11I_MODE;
wid_list[wid_cnt].type = WID_CHAR;
wid_list[wid_cnt].size = sizeof(char);
wid_list[wid_cnt].val = (s8 *)&hif_drv->usr_conn_req.security;
wid_cnt++;
- if (memcmp("DIRECT-", conn_attr->ssid, 7))
- mode_11i = hif_drv->usr_conn_req.security;
-
wid_list[wid_cnt].id = (u16)WID_AUTH_TYPE;
wid_list[wid_cnt].type = WID_CHAR;
wid_list[wid_cnt].size = sizeof(char);
wid_list[wid_cnt].val = (s8 *)&hif_drv->usr_conn_req.auth_type;
wid_cnt++;
- if (memcmp("DIRECT-", conn_attr->ssid, 7))
- auth_type = (u8)hif_drv->usr_conn_req.auth_type;
-
wid_list[wid_cnt].id = (u16)WID_JOIN_REQ_EXTENDED;
wid_list[wid_cnt].type = WID_STR;
wid_list[wid_cnt].size = 112;
wid_list[wid_cnt].val = kmalloc(wid_list[wid_cnt].size, GFP_KERNEL);
- if (memcmp("DIRECT-", conn_attr->ssid, 7)) {
- join_req_size = wid_list[wid_cnt].size;
- join_req = kmalloc(join_req_size, GFP_KERNEL);
- }
if (!wid_list[wid_cnt].val) {
result = -EFAULT;
goto error;
@@ -1120,11 +1097,6 @@ static s32 handle_connect(struct wilc_vif *vif,
cur_byte = wid_list[wid_cnt].val;
wid_cnt++;
- if (memcmp("DIRECT-", conn_attr->ssid, 7)) {
- memcpy(join_req, cur_byte, join_req_size);
- join_req_vif = vif;
- }
-
if (conn_attr->bssid)
memcpy(wilc_connected_ssid,
conn_attr->bssid, ETH_ALEN);
@@ -1254,16 +1226,6 @@ static s32 handle_connect_timeout(struct wilc_vif *vif)
eth_zero_addr(wilc_connected_ssid);
- if (join_req && join_req_vif == vif) {
- kfree(join_req);
- join_req = NULL;
- }
-
- if (info_element && join_req_vif == vif) {
- kfree(info_element);
- info_element = NULL;
- }
-
return result;
}
@@ -1519,17 +1481,6 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
hif_drv->usr_conn_req.ies_len = 0;
kfree(hif_drv->usr_conn_req.ies);
hif_drv->usr_conn_req.ies = NULL;
-
- if (join_req && join_req_vif == vif) {
- kfree(join_req);
- join_req = NULL;
- }
-
- if (info_element && join_req_vif == vif) {
- kfree(info_element);
- info_element = NULL;
- }
-
hif_drv->hif_state = HOST_IF_IDLE;
scan_while_connected = false;
@@ -1866,16 +1817,6 @@ static void handle_disconnect(struct wilc_vif *vif)
kfree(conn_req->ies);
conn_req->ies = NULL;
- if (join_req && join_req_vif == vif) {
- kfree(join_req);
- join_req = NULL;
- }
-
- if (info_element && join_req_vif == vif) {
- kfree(info_element);
- info_element = NULL;
- }
-
out:
complete(&hif_drv->comp_test_disconn_block);
--
2.7.4
On Fri, Mar 23, 2018 at 08:38:53PM +0530, Ajay Singh wrote:
> Free memory allocated for wep key when command enqueue is failed.
>
> Signed-off-by: Ajay Singh <[email protected]>
> ---
> drivers/staging/wilc1000/host_interface.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 1cc4c08..c958dd3 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -2727,8 +2727,10 @@ int wilc_add_wep_key_bss_sta(struct wilc_vif *vif, const u8 *key, u8 len,
> msg.body.key_info.attr.wep.index = index;
>
> result = wilc_enqueue_cmd(&msg);
> - if (result)
> + if (result) {
> netdev_err(vif->ndev, "STA - WEP Key\n");
> + kfree(msg.body.key_info.attr.wep.key);
We should "return result;" here otherwise we'll hang when we
wait_for_completion(). This is the sort of bug why I always encourage
people to keep the error path and success path separate (unless they
both have to unlock or free the same resources).
> + }
> wait_for_completion(&hif_drv->comp_test_key_block);
>
> return result;
That way this becomes a "return 0;" instead of a "return result;".
> @@ -2763,10 +2765,12 @@ int wilc_add_wep_key_bss_ap(struct wilc_vif *vif, const u8 *key, u8 len,
>
> result = wilc_enqueue_cmd(&msg);
>
> - if (result)
> + if (result) {
> netdev_err(vif->ndev, "AP - WEP Key\n");
> - else
> + kfree(msg.body.key_info.attr.wep.key);
> + } else {
> wait_for_completion(&hif_drv->comp_test_key_block);
> + }
>
> return result;
> }
This code works, but it would look cleaner with "return result;".
result = wilc_enqueue_cmd(&msg);
if (result) {
netdev_err(vif->ndev, "AP - WEP Key\n");
kfree(msg.body.key_info.attr.wep.key);
return result;
}
wait_for_completion(&hif_drv->comp_test_key_block);
return 0;
I removed a blank line between the wilc_enqueue_cmd() and the error
handling because they're very connected. All the success path is at
indent level one so you can just glance at the function and see what
it's supposed to do in the normal case. The error handling is self
contained at indent level two.
regards,
dan carpenter
Added fix to free the allocated memory in case of failure to enqueue
the command.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 70c10bc..1cc4c08 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3709,8 +3709,10 @@ int wilc_add_station(struct wilc_vif *vif, struct add_sta_param *sta_param)
}
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "wilc_mq_send fail\n");
+ kfree(add_sta_info->rates);
+ }
return result;
}
@@ -3793,8 +3795,10 @@ int wilc_edit_station(struct wilc_vif *vif,
}
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "wilc_mq_send fail\n");
+ kfree(add_sta_info->rates);
+ }
return result;
}
--
2.7.4
Fix 'Too many leading tabs' issue found by checkpatch.pl script in
handle_rcvd_gnrl_async_info().
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 286 ++++++++++++++++--------------
1 file changed, 149 insertions(+), 137 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 74095ec..bb13adb 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1302,6 +1302,153 @@ static s32 host_int_get_assoc_res_info(struct wilc_vif *vif,
u32 max_assoc_resp_info_len,
u32 *rcvd_assoc_resp_info_len);
+static inline void host_int_free_user_conn_req(struct host_if_drv *hif_drv)
+{
+ hif_drv->usr_conn_req.ssid_len = 0;
+ kfree(hif_drv->usr_conn_req.ssid);
+ hif_drv->usr_conn_req.ssid = NULL;
+ kfree(hif_drv->usr_conn_req.bssid);
+ hif_drv->usr_conn_req.bssid = NULL;
+ hif_drv->usr_conn_req.ies_len = 0;
+ kfree(hif_drv->usr_conn_req.ies);
+ hif_drv->usr_conn_req.ies = NULL;
+}
+
+static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
+ u8 mac_status)
+{
+ struct connect_resp_info *connect_resp_info = NULL;
+ struct connect_info conn_info;
+ struct host_if_drv *hif_drv = vif->hif_drv;
+
+ memset(&conn_info, 0, sizeof(struct connect_info));
+
+ if (mac_status == MAC_CONNECTED) {
+ u32 rcvd_assoc_resp_info_len;
+
+ memset(rcv_assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
+
+ host_int_get_assoc_res_info(vif, rcv_assoc_resp,
+ MAX_ASSOC_RESP_FRAME_SIZE,
+ &rcvd_assoc_resp_info_len);
+
+ if (rcvd_assoc_resp_info_len != 0) {
+ s32 err = 0;
+
+ err = wilc_parse_assoc_resp_info(rcv_assoc_resp, rcvd_assoc_resp_info_len,
+ &connect_resp_info);
+ if (err) {
+ netdev_err(vif->ndev,
+ "wilc_parse_assoc_resp_info() returned error %d\n",
+ err);
+ } else {
+ conn_info.status = connect_resp_info->status;
+
+ if (conn_info.status == SUCCESSFUL_STATUSCODE &&
+ connect_resp_info->ies) {
+ conn_info.resp_ies_len = connect_resp_info->ies_len;
+ conn_info.resp_ies = kmalloc(connect_resp_info->ies_len, GFP_KERNEL);
+ memcpy(conn_info.resp_ies, connect_resp_info->ies,
+ connect_resp_info->ies_len);
+ }
+
+ if (connect_resp_info) {
+ kfree(connect_resp_info->ies);
+ kfree(connect_resp_info);
+ }
+ }
+ }
+ }
+
+ if (mac_status == MAC_CONNECTED &&
+ conn_info.status != SUCCESSFUL_STATUSCODE) {
+ netdev_err(vif->ndev,
+ "Received MAC status is MAC_CONNECTED while the received status code in Asoc Resp is not SUCCESSFUL_STATUSCODE\n");
+ eth_zero_addr(wilc_connected_ssid);
+ } else if (mac_status == MAC_DISCONNECTED) {
+ netdev_err(vif->ndev, "Received MAC status is MAC_DISCONNECTED\n");
+ eth_zero_addr(wilc_connected_ssid);
+ }
+
+ if (hif_drv->usr_conn_req.bssid) {
+ memcpy(conn_info.bssid, hif_drv->usr_conn_req.bssid, 6);
+
+ if (mac_status == MAC_CONNECTED &&
+ conn_info.status == SUCCESSFUL_STATUSCODE) {
+ memcpy(hif_drv->assoc_bssid,
+ hif_drv->usr_conn_req.bssid, ETH_ALEN);
+ }
+ }
+
+ if (hif_drv->usr_conn_req.ies) {
+ conn_info.req_ies_len = hif_drv->usr_conn_req.ies_len;
+ conn_info.req_ies = kmalloc(hif_drv->usr_conn_req.ies_len,
+ GFP_KERNEL);
+ memcpy(conn_info.req_ies, hif_drv->usr_conn_req.ies,
+ hif_drv->usr_conn_req.ies_len);
+ }
+
+ del_timer(&hif_drv->connect_timer);
+ hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_CONN_RESP,
+ &conn_info, mac_status, NULL,
+ hif_drv->usr_conn_req.arg);
+
+ if (mac_status == MAC_CONNECTED &&
+ conn_info.status == SUCCESSFUL_STATUSCODE) {
+ wilc_set_power_mgmt(vif, 0, 0);
+
+ hif_drv->hif_state = HOST_IF_CONNECTED;
+
+ wilc_optaining_ip = true;
+ mod_timer(&wilc_during_ip_timer,
+ jiffies + msecs_to_jiffies(10000));
+ } else {
+ hif_drv->hif_state = HOST_IF_IDLE;
+ scan_while_connected = false;
+ }
+
+ kfree(conn_info.resp_ies);
+ conn_info.resp_ies = NULL;
+
+ kfree(conn_info.req_ies);
+ conn_info.req_ies = NULL;
+ host_int_free_user_conn_req(hif_drv);
+}
+
+static inline void host_int_handle_disconnect(struct wilc_vif *vif)
+{
+ struct disconnect_info disconn_info;
+ struct host_if_drv *hif_drv = vif->hif_drv;
+
+ memset(&disconn_info, 0, sizeof(struct disconnect_info));
+
+ if (hif_drv->usr_scan_req.scan_result) {
+ del_timer(&hif_drv->scan_timer);
+ handle_scan_done(vif, SCAN_EVENT_ABORTED);
+ }
+
+ disconn_info.reason = 0;
+ disconn_info.ie = NULL;
+ disconn_info.ie_len = 0;
+
+ if (hif_drv->usr_conn_req.conn_result) {
+ wilc_optaining_ip = false;
+ wilc_set_power_mgmt(vif, 0, 0);
+
+ hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
+ NULL, 0, &disconn_info,
+ hif_drv->usr_conn_req.arg);
+ } else {
+ netdev_err(vif->ndev, "Connect result NULL\n");
+ }
+
+ eth_zero_addr(hif_drv->assoc_bssid);
+
+ host_int_free_user_conn_req(hif_drv);
+ hif_drv->hif_state = HOST_IF_IDLE;
+ scan_while_connected = false;
+}
+
static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
struct rcvd_async_info *rcvd_info)
{
@@ -1314,9 +1461,6 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
u8 mac_status;
u8 mac_status_reason_code;
u8 mac_status_additional_info;
- struct connect_info conn_info;
- struct disconnect_info disconn_info;
- s32 err = 0;
struct host_if_drv *hif_drv = vif->hif_drv;
if (!hif_drv) {
@@ -1348,142 +1492,10 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
mac_status_reason_code = rcvd_info->buffer[8];
mac_status_additional_info = rcvd_info->buffer[9];
if (hif_drv->hif_state == HOST_IF_WAITING_CONN_RESP) {
- u32 rcvd_assoc_resp_info_len = 0;
- struct connect_resp_info *connect_resp_info = NULL;
-
- memset(&conn_info, 0, sizeof(struct connect_info));
-
- if (mac_status == MAC_CONNECTED) {
- memset(rcv_assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
-
- host_int_get_assoc_res_info(vif,
- rcv_assoc_resp,
- MAX_ASSOC_RESP_FRAME_SIZE,
- &rcvd_assoc_resp_info_len);
-
- if (rcvd_assoc_resp_info_len != 0) {
- err = wilc_parse_assoc_resp_info(rcv_assoc_resp, rcvd_assoc_resp_info_len,
- &connect_resp_info);
- if (err) {
- netdev_err(vif->ndev, "wilc_parse_assoc_resp_info() returned error %d\n", err);
- } else {
- conn_info.status = connect_resp_info->status;
-
- if (conn_info.status == SUCCESSFUL_STATUSCODE && connect_resp_info->ies) {
- conn_info.resp_ies_len = connect_resp_info->ies_len;
- conn_info.resp_ies = kmalloc(connect_resp_info->ies_len, GFP_KERNEL);
- memcpy(conn_info.resp_ies, connect_resp_info->ies,
- connect_resp_info->ies_len);
- }
-
- if (connect_resp_info) {
- kfree(connect_resp_info->ies);
- kfree(connect_resp_info);
- }
- }
- }
- }
-
- if (mac_status == MAC_CONNECTED &&
- conn_info.status != SUCCESSFUL_STATUSCODE) {
- netdev_err(vif->ndev, "Received MAC status is MAC_CONNECTED while the received status code in Asoc Resp is not SUCCESSFUL_STATUSCODE\n");
- eth_zero_addr(wilc_connected_ssid);
- } else if (mac_status == MAC_DISCONNECTED) {
- netdev_err(vif->ndev, "Received MAC status is MAC_DISCONNECTED\n");
- eth_zero_addr(wilc_connected_ssid);
- }
-
- if (hif_drv->usr_conn_req.bssid) {
- memcpy(conn_info.bssid, hif_drv->usr_conn_req.bssid, 6);
-
- if (mac_status == MAC_CONNECTED &&
- conn_info.status == SUCCESSFUL_STATUSCODE) {
- memcpy(hif_drv->assoc_bssid,
- hif_drv->usr_conn_req.bssid, ETH_ALEN);
- }
- }
-
- if (hif_drv->usr_conn_req.ies) {
- conn_info.req_ies_len = hif_drv->usr_conn_req.ies_len;
- conn_info.req_ies = kmalloc(hif_drv->usr_conn_req.ies_len, GFP_KERNEL);
- memcpy(conn_info.req_ies,
- hif_drv->usr_conn_req.ies,
- hif_drv->usr_conn_req.ies_len);
- }
-
- del_timer(&hif_drv->connect_timer);
- hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_CONN_RESP,
- &conn_info,
- mac_status,
- NULL,
- hif_drv->usr_conn_req.arg);
-
- if (mac_status == MAC_CONNECTED &&
- conn_info.status == SUCCESSFUL_STATUSCODE) {
- wilc_set_power_mgmt(vif, 0, 0);
-
- hif_drv->hif_state = HOST_IF_CONNECTED;
-
- wilc_optaining_ip = true;
- mod_timer(&wilc_during_ip_timer,
- jiffies + msecs_to_jiffies(10000));
- } else {
- hif_drv->hif_state = HOST_IF_IDLE;
- scan_while_connected = false;
- }
-
- kfree(conn_info.resp_ies);
- conn_info.resp_ies = NULL;
-
- kfree(conn_info.req_ies);
- conn_info.req_ies = NULL;
- hif_drv->usr_conn_req.ssid_len = 0;
- kfree(hif_drv->usr_conn_req.ssid);
- hif_drv->usr_conn_req.ssid = NULL;
- kfree(hif_drv->usr_conn_req.bssid);
- hif_drv->usr_conn_req.bssid = NULL;
- hif_drv->usr_conn_req.ies_len = 0;
- kfree(hif_drv->usr_conn_req.ies);
- hif_drv->usr_conn_req.ies = NULL;
+ host_int_parse_assoc_resp_info(vif, mac_status);
} else if ((mac_status == MAC_DISCONNECTED) &&
(hif_drv->hif_state == HOST_IF_CONNECTED)) {
- memset(&disconn_info, 0, sizeof(struct disconnect_info));
-
- if (hif_drv->usr_scan_req.scan_result) {
- del_timer(&hif_drv->scan_timer);
- handle_scan_done(vif, SCAN_EVENT_ABORTED);
- }
-
- disconn_info.reason = 0;
- disconn_info.ie = NULL;
- disconn_info.ie_len = 0;
-
- if (hif_drv->usr_conn_req.conn_result) {
- wilc_optaining_ip = false;
- wilc_set_power_mgmt(vif, 0, 0);
-
- hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF,
- NULL,
- 0,
- &disconn_info,
- hif_drv->usr_conn_req.arg);
- } else {
- netdev_err(vif->ndev, "Connect result NULL\n");
- }
-
- eth_zero_addr(hif_drv->assoc_bssid);
-
- hif_drv->usr_conn_req.ssid_len = 0;
- kfree(hif_drv->usr_conn_req.ssid);
- hif_drv->usr_conn_req.ssid = NULL;
- kfree(hif_drv->usr_conn_req.bssid);
- hif_drv->usr_conn_req.bssid = NULL;
- hif_drv->usr_conn_req.ies_len = 0;
- kfree(hif_drv->usr_conn_req.ies);
- hif_drv->usr_conn_req.ies = NULL;
- hif_drv->hif_state = HOST_IF_IDLE;
- scan_while_connected = false;
-
+ host_int_handle_disconnect(vif);
} else if ((mac_status == MAC_DISCONNECTED) &&
(hif_drv->usr_scan_req.scan_result)) {
del_timer(&hif_drv->scan_timer);
--
2.7.4
Reviewed-by: Claudiu Beznea <[email protected]>
On 23.03.2018 17:08, Ajay Singh wrote:
> This patch series contains changes to fix memory leaks, avoid NULL pointer
> exceptions and checkpatch reported issue fixes.
>
> Ajay Singh (9):
> staging: wilc1000: remove unused global variables related to p2p
> staging: wilc1000: avoid 'NULL' pointer access in
> wilc_network_info_received()
> staging: wilc1000: free allocated memory in edit and add station
> functions
> staging: wilc1000: free memory allocated in add wep key functions
> staging: wilc1000: free allocated memory after processing
> wilc_send_config_pkt()
> staging: wilc1000: fix to free allocated memory in wilc_add_ptk()
> staging: wilc1000: free allocated memory in wilc_add_rx_gtk()
> staging: wilc1000: split handle_rcvd_gnrl_async_info() to avoid
> leading tabs
> staging: wilc1000: free memory allocated for general info message from
> firmware
>
> drivers/staging/wilc1000/host_interface.c | 421 +++++++++++++++---------------
> 1 file changed, 204 insertions(+), 217 deletions(-)
>
Free memory allocated in wilc_add_rx_gtk() before returing from the
function.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index a8771d5..74095ec 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2881,8 +2881,10 @@ int wilc_add_rx_gtk(struct wilc_vif *vif, const u8 *rx_gtk, u8 gtk_key_len,
msg.body.key_info.attr.wpa.key = kmemdup(rx_gtk,
key_len,
GFP_KERNEL);
- if (!msg.body.key_info.attr.wpa.key)
+ if (!msg.body.key_info.attr.wpa.key) {
+ kfree(msg.body.key_info.attr.wpa.seq);
return -ENOMEM;
+ }
if (rx_mic)
memcpy(msg.body.key_info.attr.wpa.key + 16, rx_mic,
@@ -2897,10 +2899,13 @@ int wilc_add_rx_gtk(struct wilc_vif *vif, const u8 *rx_gtk, u8 gtk_key_len,
msg.body.key_info.attr.wpa.seq_len = key_rsc_len;
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "RX GTK\n");
- else
+ kfree(msg.body.key_info.attr.wpa.seq);
+ kfree(msg.body.key_info.attr.wpa.key);
+ } else {
wait_for_completion(&hif_drv->comp_test_key_block);
+ }
return result;
}
--
2.7.4
Free allocated memory for failure scenario while processing the
information message received from the firmware. Added NULL check and used
kmemdup in the flow of handling information message.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 48 ++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index bb13adb..d35885b 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1346,16 +1346,15 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
if (conn_info.status == SUCCESSFUL_STATUSCODE &&
connect_resp_info->ies) {
- conn_info.resp_ies_len = connect_resp_info->ies_len;
- conn_info.resp_ies = kmalloc(connect_resp_info->ies_len, GFP_KERNEL);
- memcpy(conn_info.resp_ies, connect_resp_info->ies,
- connect_resp_info->ies_len);
+ conn_info.resp_ies = kmemdup(connect_resp_info->ies,
+ connect_resp_info->ies_len,
+ GFP_KERNEL);
+ if (conn_info.resp_ies)
+ conn_info.resp_ies_len = connect_resp_info->ies_len;
}
- if (connect_resp_info) {
- kfree(connect_resp_info->ies);
- kfree(connect_resp_info);
- }
+ kfree(connect_resp_info->ies);
+ kfree(connect_resp_info);
}
}
}
@@ -1381,11 +1380,11 @@ static inline void host_int_parse_assoc_resp_info(struct wilc_vif *vif,
}
if (hif_drv->usr_conn_req.ies) {
- conn_info.req_ies_len = hif_drv->usr_conn_req.ies_len;
- conn_info.req_ies = kmalloc(hif_drv->usr_conn_req.ies_len,
+ conn_info.req_ies = kmemdup(conn_info.req_ies,
+ hif_drv->usr_conn_req.ies_len,
GFP_KERNEL);
- memcpy(conn_info.req_ies, hif_drv->usr_conn_req.ies,
- hif_drv->usr_conn_req.ies_len);
+ if (conn_info.req_ies)
+ conn_info.req_ies_len = hif_drv->usr_conn_req.ies_len;
}
del_timer(&hif_drv->connect_timer);
@@ -1463,17 +1462,25 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
u8 mac_status_additional_info;
struct host_if_drv *hif_drv = vif->hif_drv;
+ if (!rcvd_info->buffer) {
+ netdev_err(vif->ndev, "Received buffer is NULL\n");
+ return -EINVAL;
+ }
+
if (!hif_drv) {
netdev_err(vif->ndev, "Driver handler is NULL\n");
+ kfree(rcvd_info->buffer);
+ rcvd_info->buffer = NULL;
return -ENODEV;
}
if (hif_drv->hif_state == HOST_IF_WAITING_CONN_RESP ||
hif_drv->hif_state == HOST_IF_CONNECTED ||
hif_drv->usr_scan_req.scan_result) {
- if (!rcvd_info->buffer ||
- !hif_drv->usr_conn_req.conn_result) {
+ if (!hif_drv->usr_conn_req.conn_result) {
netdev_err(vif->ndev, "driver is null\n");
+ kfree(rcvd_info->buffer);
+ rcvd_info->buffer = NULL;
return -EINVAL;
}
@@ -1481,6 +1488,8 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif *vif,
if ('I' != msg_type) {
netdev_err(vif->ndev, "Received Message incorrect.\n");
+ kfree(rcvd_info->buffer);
+ rcvd_info->buffer = NULL;
return -EFAULT;
}
@@ -3528,12 +3537,17 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
msg.vif = vif;
msg.body.async_info.len = length;
- msg.body.async_info.buffer = kmalloc(length, GFP_KERNEL);
- memcpy(msg.body.async_info.buffer, buffer, length);
+ msg.body.async_info.buffer = kmemdup(buffer, length, GFP_KERNEL);
+ if (!msg.body.async_info.buffer) {
+ mutex_unlock(&hif_deinit_lock);
+ return;
+ }
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "synchronous info (%d)\n", result);
+ kfree(msg.body.async_info.buffer);
+ }
mutex_unlock(&hif_deinit_lock);
}
--
2.7.4
Hi Dan,
On Mon, 26 Mar 2018 11:17:48 +0300
Dan Carpenter <[email protected]> wrote:
> On Fri, Mar 23, 2018 at 08:38:53PM +0530, Ajay Singh wrote:
> We should "return result;" here otherwise we'll hang when we
> wait_for_completion(). This is the sort of bug why I always encourage
> people to keep the error path and success path separate (unless they
> both have to unlock or free the same resources).
>
Yes, wait_for_completion() will hang for the error path. I have included
the changes in V2 patch series.
>
> This code works, but it would look cleaner with "return result;".
>
> result = wilc_enqueue_cmd(&msg);
> if (result) {
> netdev_err(vif->ndev, "AP - WEP Key\n");
> kfree(msg.body.key_info.attr.wep.key);
> return result;
> }
>
> wait_for_completion(&hif_drv->comp_test_key_block);
> return 0;
>
> I removed a blank line between the wilc_enqueue_cmd() and the error
> handling because they're very connected. All the success path is at
> indent level one so you can just glance at the function and see what
> it's supposed to do in the normal case. The error handling is self
> contained at indent level two.
>
I will send the updated patch by modifying the code as suggested.
Regards,
Ajay
Added 'NULL' check before accessing the allocated memory. Free up the
memory incase of failure to enqueue the command. Used kmemdup instead of
kmalloc & memcpy.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index a13998d..70c10bc 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3453,12 +3453,15 @@ void wilc_network_info_received(struct wilc *wilc, u8 *buffer, u32 length)
msg.vif = vif;
msg.body.net_info.len = length;
- msg.body.net_info.buffer = kmalloc(length, GFP_KERNEL);
- memcpy(msg.body.net_info.buffer, buffer, length);
+ msg.body.net_info.buffer = kmemdup(buffer, length, GFP_KERNEL);
+ if (!msg.body.net_info.buffer)
+ return;
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "message parameters (%d)\n", result);
+ kfree(msg.body.net_info.buffer);
+ }
}
void wilc_gnrl_async_info_received(struct wilc *wilc, u8 *buffer, u32 length)
--
2.7.4
Free memory allocated for wep key when command enqueue is failed.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 1cc4c08..c958dd3 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2727,8 +2727,10 @@ int wilc_add_wep_key_bss_sta(struct wilc_vif *vif, const u8 *key, u8 len,
msg.body.key_info.attr.wep.index = index;
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "STA - WEP Key\n");
+ kfree(msg.body.key_info.attr.wep.key);
+ }
wait_for_completion(&hif_drv->comp_test_key_block);
return result;
@@ -2763,10 +2765,12 @@ int wilc_add_wep_key_bss_ap(struct wilc_vif *vif, const u8 *key, u8 len,
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "AP - WEP Key\n");
- else
+ kfree(msg.body.key_info.attr.wep.key);
+ } else {
wait_for_completion(&hif_drv->comp_test_key_block);
+ }
return result;
}
--
2.7.4
Free allocated memory in wilc_add_ptk() when it fails to enqueue the
command.
Signed-off-by: Ajay Singh <[email protected]>
---
drivers/staging/wilc1000/host_interface.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index f493c78..a8771d5 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2827,10 +2827,12 @@ int wilc_add_ptk(struct wilc_vif *vif, const u8 *ptk, u8 ptk_key_len,
result = wilc_enqueue_cmd(&msg);
- if (result)
+ if (result) {
netdev_err(vif->ndev, "PTK Key\n");
- else
+ kfree(msg.body.key_info.attr.wpa.key);
+ } else {
wait_for_completion(&hif_drv->comp_test_key_block);
+ }
return result;
}
--
2.7.4
What happened to patch 8/9? Anyway, I can't apply this patch and it
could be my fault or it could be the missing patch. I don't know...
Anwyway, seems like a nice patchset.
regards,
dan carpenter
On Mon, Mar 26, 2018 at 05:01:50PM +0530, Ajay Singh wrote:
> On Mon, 26 Mar 2018 11:32:41 +0300
> Dan Carpenter <[email protected]> wrote:
>
> > What happened to patch 8/9? Anyway, I can't apply this patch and it
> > could be my fault or it could be the missing patch. I don't know...
>
> I rechecked by applying the patches in order and didn't face any conflict.
> I am going to send the v2 for this patch series by including the review
> comments.
The problem was on my end. Sorry. Gmail's spam filtering messed up.
I should have checked better.
regards,
dan carpenter