| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f674c9e0-e67f-885f-cdd8-ce5da1fcc262@microchip.com>
Date: Tue, 27 Mar 2018 11:44:18 +0300
From: Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To: Colin King <colin.king@...onical.com>,
Aditya Shankar <aditya.shankar@...rochip.com>,
Ganesh Krishna <ganesh.krishna@...rochip.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
<linux-wireless@...r.kernel.org>, <devel@...verdev.osuosl.org>
CC: <kernel-janitors@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] staging: wilc1000: replace kmalloc + memcpy with kmemdup
On 26.03.2018 20:16, Colin King wrote:
> From: Colin Ian King <colin.king@...onical.com>
>
> Replace several allocation and memcpys with kmemdup and add in some
> missing memory allocation failure checks. Also fix an incorrect
> -EFAULT return with -ENOMEM.
>
> Signed-off-by: Colin Ian King <colin.king@...onical.com>
> ---
> drivers/staging/wilc1000/host_interface.c | 75 +++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 9b9b86654958..8fd367f87fa5 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -797,6 +797,10 @@ static s32 handle_scan(struct wilc_vif *vif, struct scan_attr *scan_info)
> for (i = 0; i < hidden_net->n_ssids; i++)
> valuesize += ((hidden_net->net_info[i].ssid_len) + 1);
> hdn_ntwk_wid_val = kmalloc(valuesize + 1, GFP_KERNEL);
> + if (!hdn_ntwk_wid_val) {
> + result = -ENOMEM;
> + goto error;
> + }
> wid_list[index].val = hdn_ntwk_wid_val;
> if (wid_list[index].val) {
> buffer = wid_list[index].val;
> @@ -943,39 +947,35 @@ static s32 handle_connect(struct wilc_vif *vif,
> }
>
> if (conn_attr->bssid) {
These:
> - hif_drv->usr_conn_req.bssid = kmalloc(6, GFP_KERNEL);
> + hif_drv->usr_conn_req.bssid = kmemdup(conn_attr->bssid, 6,
> + GFP_KERNEL);
> if (!hif_drv->usr_conn_req.bssid) {
> result = -ENOMEM;
> goto error;
> }
> - memcpy(hif_drv->usr_conn_req.bssid, conn_attr->bssid, 6);
> }
>
> hif_drv->usr_conn_req.ssid_len = conn_attr->ssid_len;
> if (conn_attr->ssid) {
> - hif_drv->usr_conn_req.ssid = kmalloc(conn_attr->ssid_len + 1,
> + hif_drv->usr_conn_req.ssid = kmemdup(conn_attr->ssid,
> + conn_attr->ssid_len + 1,
> GFP_KERNEL);
> if (!hif_drv->usr_conn_req.ssid) {
> result = -ENOMEM;
> goto error;
> }
> - memcpy(hif_drv->usr_conn_req.ssid,
> - conn_attr->ssid,
> - conn_attr->ssid_len);
> hif_drv->usr_conn_req.ssid[conn_attr->ssid_len] = '\0';
> }
>
> hif_drv->usr_conn_req.ies_len = conn_attr->ies_len;
> if (conn_attr->ies) {
and these:
> - hif_drv->usr_conn_req.ies = kmalloc(conn_attr->ies_len,
> + hif_drv->usr_conn_req.ies = kmemdup(conn_attr->ies,
> + conn_attr->ies_len,
> GFP_KERNEL);
were already done in [1]. I don't know which was the first...
[1] https://patchwork.kernel.org/patch/10308575/
> if (!hif_drv->usr_conn_req.ies) {
> result = -ENOMEM;
> goto error;
> }
> - memcpy(hif_drv->usr_conn_req.ies,
> - conn_attr->ies,
> - conn_attr->ies_len);
> }
>
> hif_drv->usr_conn_req.security = conn_attr->security;
> @@ -1009,9 +1009,12 @@ static s32 handle_connect(struct wilc_vif *vif,
>
> 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);
> + info_element = kmemdup(hif_drv->usr_conn_req.ies,
> + info_element_size, GFP_KERNEL);
> + if (!info_element) {
> + result = -ENOMEM;
> + goto error;
> + }
> }
> wid_list[wid_cnt].id = (u16)WID_11I_MODE;
> wid_list[wid_cnt].type = WID_CHAR;
> @@ -1039,9 +1042,13 @@ static s32 handle_connect(struct wilc_vif *vif,
> if (memcmp("DIRECT-", conn_attr->ssid, 7)) {
> join_req_size = wid_list[wid_cnt].size;
> join_req = kmalloc(join_req_size, GFP_KERNEL);
> + if (!join_req) {
> + result = -ENOMEM;
> + goto error;
> + }
> }
> if (!wid_list[wid_cnt].val) {
> - result = -EFAULT;
> + result = -ENOMEM;
> goto error;
> }
>
> @@ -1166,11 +1173,13 @@ static s32 handle_connect(struct wilc_vif *vif,
>
> if (conn_attr->ies) {
> conn_info.req_ies_len = conn_attr->ies_len;
> - conn_info.req_ies = kmalloc(conn_attr->ies_len,
> + conn_info.req_ies = kmemdup(conn_attr->ies,
> + conn_attr->ies_len,
> GFP_KERNEL);
> - memcpy(conn_info.req_ies,
> - conn_attr->ies,
> - conn_attr->ies_len);
> + if (!conn_info.req_ies) {
> + result = -ENOMEM;
> + goto error_free;
> + }
> }
>
> conn_attr->result(CONN_DISCONN_EVENT_CONN_RESP,
> @@ -1187,6 +1196,7 @@ static s32 handle_connect(struct wilc_vif *vif,
> }
> }
>
> +error_free:
> kfree(conn_attr->bssid);
> conn_attr->bssid = NULL;
>
> @@ -1227,10 +1237,9 @@ static s32 handle_connect_timeout(struct wilc_vif *vif)
>
> if (hif_drv->usr_conn_req.ies) {
> info.req_ies_len = hif_drv->usr_conn_req.ies_len;
> - info.req_ies = kmalloc(hif_drv->usr_conn_req.ies_len, GFP_KERNEL);
> - memcpy(info.req_ies,
> - hif_drv->usr_conn_req.ies,
> - hif_drv->usr_conn_req.ies_len);
> + info.req_ies = kmemdup(hif_drv->usr_conn_req.ies,
> + hif_drv->usr_conn_req.ies_len,
> + GFP_KERNEL);
> }
>
> hif_drv->usr_conn_req.conn_result(CONN_DISCONN_EVENT_CONN_RESP,
> @@ -1421,9 +1430,11 @@ static s32 handle_rcvd_gnrl_async_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)
> + return -ENOMEM;
> }
>
> if (connect_resp_info) {
> @@ -3524,8 +3535,11 @@ 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) {
> + netdev_err(vif->ndev, "message buffer allocation failed\n");
> + return;
> + }
>
> result = wilc_enqueue_cmd(&msg);
> if (result)
> @@ -3571,8 +3585,11 @@ 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) {
> + netdev_err(vif->ndev, "message buffer allocation failed\n");
> + return;
> + }
>
> result = wilc_enqueue_cmd(&msg);
> if (result)
>
Powered by blists - more mailing lists