From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELtjY+wYB8At+M9E+XmFOAQIyku571mI8hgYo+OTQF6C7SL7KRBrBqc8T7Pt1QVT/ahnpdrA ARC-Seal: i=1; a=rsa-sha256; t=1522140262; cv=none; d=google.com; s=arc-20160816; b=bsrn9+DsAQxMSUzJKPqMiOotw3wqkOvP2obye1nqTnfukR2v2Vm1I+M6XKH4E/qmrb Ykj0vAmnjCPHWTV5OeP6VEqaEFqzCGX6//Gv+2Jt1z06M/fwc6tyEKpgdu1wH+mdoSFC r3f+T2+BXUi0JflBjVpX6VWixR0bYDkqCiVyebFrvtt8JolKzJNNLFs2hXPvup26BwsS EtRwB+rFXa97+kWSADwoxJnwH397a9ZboerZY7XLgPGKa4t8QKAxE9qXEFWjOv/uQQc5 Uh7SNhkPVyu0Ul1jJi28CQ/4F1rfJK0yvyQq1LSA88DJcEPsfAKTkoDJBc+4FI4lArnH RHXQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :arc-authentication-results; bh=9VQsJ8FC6jbPF9rPOahYJZKJ6oEI7th9OWjqN0X/URA=; b=NpxEI8KZ+muIjqcyDWdWX3R+sxs6GGPRcqY2SsUM8oCzN7E65duFOKBPgA59CWLYmd KkXDQSYU+vmgo6+l/IlPgJBLbV3Z9rzxv8zZtfpGIlLydOos897s9ilTIuHd5EyiXZaj l7iioBwtpmx13oWFot5RyuE4UzKW+FEcBXF1StSP2A22G5+LCEAla1E47GZy3rMHeazs 06jSZ+YMzVdoqwuZR6Aik/Q8ZPObq80OsNCot6OaorXBcWUUuCzQpbU6sMydHdTMR8E9 fq8DUER3AnrNFCWd8FTK54vGicOrApCRkCJc7I1ls4pARBebjHwWqR82VlF44/d/XXS7 m2dA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of claudiu.beznea@microchip.com designates 68.232.154.123 as permitted sender) smtp.mailfrom=Claudiu.Beznea@microchip.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of claudiu.beznea@microchip.com designates 68.232.154.123 as permitted sender) smtp.mailfrom=Claudiu.Beznea@microchip.com X-IronPort-AV: E=Sophos;i="5.48,366,1517900400"; d="scan'208";a="12430580" Subject: Re: [PATCH] staging: wilc1000: replace kmalloc + memcpy with kmemdup To: Colin King , Aditya Shankar , Ganesh Krishna , Greg Kroah-Hartman , , CC: , References: <20180326171629.28700-1-colin.king@canonical.com> From: Claudiu Beznea Message-ID: Date: Tue, 27 Mar 2018 11:44:18 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180326171629.28700-1-colin.king@canonical.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1596021371817996393?= X-GMAIL-MSGID: =?utf-8?q?1596079747772562130?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On 26.03.2018 20:16, Colin King wrote: > From: Colin Ian King > > 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 > --- > 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) >