2014-05-26 15:24:39

by Benoit Taine

Subject: [PATCH 0/18] Use kmemdup instead of kmalloc + memcpy

These patches enhance kernel style usage, and allows smaller code while
preventing accidental code edits to produce overflows.

The semantic patch at scripts/coccinelle/api/memdup.cocci was used to
detect and edit this situation.

2014-05-26 15:56:56

by Benoit Taine

Subject: Re: [PATCH 9/18] staging: rtl8723au: Use kmemdup instead of kmalloc + memcpy

On 26/05/2014 17:43, Jes Sorensen wrote:
> Benoit Taine <[email protected]> writes:
> > This issue was reported by coccicheck using the semantic patch
> > at scripts/coccinelle/api/memdup.cocci
> >
> > Signed-off-by: Benoit Taine <[email protected]>
> > ---
> > Tested by compilation without errors.
> >
> > drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 3 +--
> > drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 16 ++++++----------
> > 2 files changed, 7 insertions(+), 12 deletions(-)
>
> Benoit,
>
> I believe this was already applied to staging-next - if you feel
> something is missing. Could you please re-diff against the latest
> staging-next tree.

Agreed, it is mostly 80 chars per line compliance. I will resend.

--
Benoît Taine
Master cycle intern
Regal Team. LIP6

2014-05-26 15:43:56

by Jes Sorensen

Subject: Re: [PATCH 9/18] staging: rtl8723au: Use kmemdup instead of kmalloc + memcpy

Benoit Taine <[email protected]> writes:
> This issue was reported by coccicheck using the semantic patch
> at scripts/coccinelle/api/memdup.cocci
>
> Signed-off-by: Benoit Taine <[email protected]>
> ---
> Tested by compilation without errors.
>
> drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 3 +--
> drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 16 ++++++----------
> 2 files changed, 7 insertions(+), 12 deletions(-)

Benoit,

I believe this was already applied to staging-next - if you feel
something is missing. Could you please re-diff against the latest
staging-next tree.

Cheers,
Jes

commit 4a6eea4dcbc0328c3126fed264c42b0725bea659
Author: Benoit Taine <[email protected]>
Date: Thu May 22 15:08:33 2014 +0200

staging: rtl8723au: Use kmemdup() instead of memcpy() to duplicate memory

This issue was reported by coccicheck using the semantic patch
at scripts/coccinelle/api/memdup.cocci, and tested by compilation.

Signed-off-by: Benoit Taine <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


>
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> index e2d426a..f917edd 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> @@ -339,12 +339,11 @@ int rtl8723a_FirmwareDownload(struct rtw_adapter *padapter)
> rtStatus = _FAIL;
> goto Exit;
> }
> - firmware_buf = kzalloc(fw->size, GFP_KERNEL);
> + firmware_buf = kmemdup(fw->data, fw->size, GFP_KERNEL);
> if (!firmware_buf) {
> rtStatus = _FAIL;
> goto Exit;
> }
> - memcpy(firmware_buf, fw->data, fw->size);
> buf = firmware_buf;
> fw_size = fw->size;
> release_firmware(fw);
> diff --git a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
> index 182f57c..735eb99 100644
> --- a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
> +++ b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
> @@ -1426,14 +1426,13 @@ static int rtw_cfg80211_set_probe_req_wpsp2pie(struct rtw_adapter *padapter,
> pmlmepriv->wps_probe_req_ie = NULL;
> }
>
> - pmlmepriv->wps_probe_req_ie =
> - kmalloc(wps_ielen, GFP_KERNEL);
> + pmlmepriv->wps_probe_req_ie = kmemdup(wps_ie,
> + wps_ielen, GFP_KERNEL);
> if (pmlmepriv->wps_probe_req_ie == NULL) {
> DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
> __func__, __LINE__);
> return -EINVAL;
> }
> - memcpy(pmlmepriv->wps_probe_req_ie, wps_ie, wps_ielen);
> pmlmepriv->wps_probe_req_ie_len = wps_ielen;
> }
> }
> @@ -1697,12 +1696,11 @@ static int rtw_cfg80211_set_wpa_ie(struct rtw_adapter *padapter, const u8 *pie,
> ret = -EINVAL;
> goto exit;
> }
> - buf = kzalloc(ielen, GFP_KERNEL);
> + buf = kmemdup(pie, ielen, GFP_KERNEL);
> if (buf == NULL) {
> ret = -ENOMEM;
> goto exit;
> }
> - memcpy(buf, pie, ielen);
>
> /* dump */
> DBG_8723A("set wpa_ie(length:%zu):\n", ielen);
> @@ -3178,14 +3176,13 @@ static int rtw_cfg80211_set_beacon_wpsp2pie(struct net_device *ndev, char *buf,
> pmlmepriv->wps_beacon_ie = NULL;
> }
>
> - pmlmepriv->wps_beacon_ie =
> - kmalloc(wps_ielen, GFP_KERNEL);
> + pmlmepriv->wps_beacon_ie = kmemdup(wps_ie, wps_ielen,
> + GFP_KERNEL);
> if (pmlmepriv->wps_beacon_ie == NULL) {
> DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
> __func__, __LINE__);
> return -EINVAL;
> }
> - memcpy(pmlmepriv->wps_beacon_ie, wps_ie, wps_ielen);
> pmlmepriv->wps_beacon_ie_len = wps_ielen;
>
> #ifdef CONFIG_8723AU_AP_MODE
> @@ -3270,14 +3267,13 @@ static int rtw_cfg80211_set_assoc_resp_wpsp2pie(struct net_device *net,
> pmlmepriv->wps_assoc_resp_ie = NULL;
> }
>
> - pmlmepriv->wps_assoc_resp_ie = kmalloc(len, GFP_KERNEL);
> + pmlmepriv->wps_assoc_resp_ie = kmemdup(buf, len, GFP_KERNEL);
> if (pmlmepriv->wps_assoc_resp_ie == NULL) {
> DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
> __func__, __LINE__);
> return -EINVAL;
>
> }
> - memcpy(pmlmepriv->wps_assoc_resp_ie, buf, len);
> pmlmepriv->wps_assoc_resp_ie_len = len;
> }
>

2014-05-26 15:21:16

by Benoit Taine

Subject: [PATCH 9/18] staging: rtl8723au: Use kmemdup instead of kmalloc + memcpy

This issue was reported by coccicheck using the semantic patch
at scripts/coccinelle/api/memdup.cocci

Signed-off-by: Benoit Taine <[email protected]>
---
Tested by compilation without errors.

drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 3 +--
drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c | 16 ++++++----------
2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
index e2d426a..f917edd 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
@@ -339,12 +339,11 @@ int rtl8723a_FirmwareDownload(struct rtw_adapter *padapter)
rtStatus = _FAIL;
goto Exit;
}
- firmware_buf = kzalloc(fw->size, GFP_KERNEL);
+ firmware_buf = kmemdup(fw->data, fw->size, GFP_KERNEL);
if (!firmware_buf) {
rtStatus = _FAIL;
goto Exit;
}
- memcpy(firmware_buf, fw->data, fw->size);
buf = firmware_buf;
fw_size = fw->size;
release_firmware(fw);
diff --git a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
index 182f57c..735eb99 100644
--- a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
@@ -1426,14 +1426,13 @@ static int rtw_cfg80211_set_probe_req_wpsp2pie(struct rtw_adapter *padapter,
pmlmepriv->wps_probe_req_ie = NULL;
}

- pmlmepriv->wps_probe_req_ie =
- kmalloc(wps_ielen, GFP_KERNEL);
+ pmlmepriv->wps_probe_req_ie = kmemdup(wps_ie,
+ wps_ielen, GFP_KERNEL);
if (pmlmepriv->wps_probe_req_ie == NULL) {
DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
__func__, __LINE__);
return -EINVAL;
}
- memcpy(pmlmepriv->wps_probe_req_ie, wps_ie, wps_ielen);
pmlmepriv->wps_probe_req_ie_len = wps_ielen;
}
}
@@ -1697,12 +1696,11 @@ static int rtw_cfg80211_set_wpa_ie(struct rtw_adapter *padapter, const u8 *pie,
ret = -EINVAL;
goto exit;
}
- buf = kzalloc(ielen, GFP_KERNEL);
+ buf = kmemdup(pie, ielen, GFP_KERNEL);
if (buf == NULL) {
ret = -ENOMEM;
goto exit;
}
- memcpy(buf, pie, ielen);

/* dump */
DBG_8723A("set wpa_ie(length:%zu):\n", ielen);
@@ -3178,14 +3176,13 @@ static int rtw_cfg80211_set_beacon_wpsp2pie(struct net_device *ndev, char *buf,
pmlmepriv->wps_beacon_ie = NULL;
}

- pmlmepriv->wps_beacon_ie =
- kmalloc(wps_ielen, GFP_KERNEL);
+ pmlmepriv->wps_beacon_ie = kmemdup(wps_ie, wps_ielen,
+ GFP_KERNEL);
if (pmlmepriv->wps_beacon_ie == NULL) {
DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
__func__, __LINE__);
return -EINVAL;
}
- memcpy(pmlmepriv->wps_beacon_ie, wps_ie, wps_ielen);
pmlmepriv->wps_beacon_ie_len = wps_ielen;

#ifdef CONFIG_8723AU_AP_MODE
@@ -3270,14 +3267,13 @@ static int rtw_cfg80211_set_assoc_resp_wpsp2pie(struct net_device *net,
pmlmepriv->wps_assoc_resp_ie = NULL;
}

- pmlmepriv->wps_assoc_resp_ie = kmalloc(len, GFP_KERNEL);
+ pmlmepriv->wps_assoc_resp_ie = kmemdup(buf, len, GFP_KERNEL);
if (pmlmepriv->wps_assoc_resp_ie == NULL) {
DBG_8723A("%s()-%d: kmalloc() ERROR!\n",
__func__, __LINE__);
return -EINVAL;

}
- memcpy(pmlmepriv->wps_assoc_resp_ie, buf, len);
pmlmepriv->wps_assoc_resp_ie_len = len;
}

2014-05-26 15:24:25

by Benoit Taine

Subject: [PATCH 5/18] wcn36xx: Use kmemdup instead of kmalloc + memcpy

This issue was reported by coccicheck using the semantic patch
at scripts/coccinelle/api/memdup.cocci

Signed-off-by: Benoit Taine <[email protected]>
---
Tested by compilation without errors.

drivers/net/wireless/ath/wcn36xx/smd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 7bf0ef8..6398693 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2068,7 +2068,7 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
if (!msg_ind)
goto nomem;
msg_ind->msg_len = len;
- msg_ind->msg = kmalloc(len, GFP_KERNEL);
+ msg_ind->msg = kmemdup(buf, len, GFP_KERNEL);
if (!msg_ind->msg) {
kfree(msg_ind);
nomem:
@@ -2080,7 +2080,6 @@ nomem:
msg_header->msg_type);
break;
}
- memcpy(msg_ind->msg, buf, len);
mutex_lock(&wcn->hal_ind_mutex);
list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);