This is a cleanup of staging/rtl8188eu/os_dep/ioctl_linux.c regarding Coverity
resource leak findings.
The changes leave the current implementation intact and just attack the problems
in the error paths, however, it seems that we could get easily rid of some of
the mallocs altogether.
char *input = kmalloc(wrqu->data.length, GFP_KERNEL);
copy_from_user(input, wrqu->data.pointer, wrqu->data.length);
qAutoLoad = strncmp(input, "autoload", 8);
The series is compile tested and applies against branch staging-next of tree
git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
Christian Engelmayer (5):
staging: rtl8188eu: fix potential leak in rtw_wx_read32()
staging: rtl8188eu: fix potential leak in rtw_wx_set_enc_ext()
staging: rtl8188eu: fix potential leak in rtw_mp_QueryDrv()
staging: rtl8188eu: fix potential leak in rtw_mp_SetRFPath()
staging: rtl8188eu: fix potential leak in rtw_mp_pwrtrk()
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 69 +++++++++++++++++---------
1 file changed, 45 insertions(+), 24 deletions(-)
--
1.9.1
Function rtw_wx_read32() dynamically allocates a temporary buffer that is not
freed in all error paths. Use a centralized exit path and make sure that all
memory is freed correctly. Detected by Coverity - CID 1077711.
Signed-off-by: Christian Engelmayer <[email protected]>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index cf30a08..45b47e2 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -2154,6 +2154,7 @@ static int rtw_wx_read32(struct net_device *dev,
u32 bytes;
u8 *ptmp;
int rv;
+ int ret = 0;
padapter = (struct adapter *)rtw_netdev_priv(dev);
p = &wrqu->data;
@@ -2163,16 +2164,16 @@ static int rtw_wx_read32(struct net_device *dev,
return -ENOMEM;
if (copy_from_user(ptmp, p->pointer, len)) {
- kfree(ptmp);
- return -EFAULT;
+ ret = -EFAULT;
+ goto exit;
}
bytes = 0;
addr = 0;
rv = sscanf(ptmp, "%d,%x", &bytes, &addr);
if (rv != 2) {
- kfree(ptmp);
- return -EINVAL;
+ ret = -EINVAL;
+ goto exit;
}
switch (bytes) {
@@ -2190,12 +2191,14 @@ static int rtw_wx_read32(struct net_device *dev,
break;
default:
DBG_88E(KERN_INFO "%s: usage> read [bytes],[address(hex)]\n", __func__);
- return -EINVAL;
+ ret = -EINVAL;
+ goto exit;
}
DBG_88E(KERN_INFO "%s: addr = 0x%08X data =%s\n", __func__, addr, extra);
+exit:
kfree(ptmp);
- return 0;
+ return ret;
}
static int rtw_wx_write32(struct net_device *dev,
--
1.9.1
Function rtw_mp_pwrtrk() dynamically allocates a temporary buffer that
is not freed in all error paths. Use a centralized exit path and make sure
that all memory is freed correctly. Detected by Coverity - 1077715.
Signed-off-by: Christian Engelmayer <[email protected]>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 28 ++++++++++++++++----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index ea5e1f8..f04aaa3 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -7119,15 +7119,15 @@ static int rtw_mp_pwrtrk(struct net_device *dev,
{
u8 enable;
u32 thermal;
- s32 ret;
struct adapter *padapter = rtw_netdev_priv(dev);
char *input = kmalloc(wrqu->length, GFP_KERNEL);
+ int ret = 0;
if (!input)
return -ENOMEM;
if (copy_from_user(input, wrqu->pointer, wrqu->length)) {
- kfree(input);
- return -EFAULT;
+ ret = -EFAULT;
+ goto exit;
}
_rtw_memset(extra, 0, wrqu->length);
@@ -7138,22 +7138,28 @@ static int rtw_mp_pwrtrk(struct net_device *dev,
sprintf(extra, "mp tx power tracking stop");
} else if (sscanf(input, "ther =%d", &thermal)) {
ret = Hal_SetThermalMeter(padapter, (u8)thermal);
- if (ret == _FAIL)
- return -EPERM;
+ if (ret == _FAIL) {
+ ret = -EPERM;
+ goto exit;
+ }
sprintf(extra, "mp tx power tracking start, target value =%d ok ", thermal);
} else {
- kfree(input);
- return -EINVAL;
+ ret = -EINVAL;
+ goto exit;
}
}
- kfree(input);
ret = Hal_SetPowerTracking(padapter, enable);
- if (ret == _FAIL)
- return -EPERM;
+ if (ret == _FAIL) {
+ ret = -EPERM;
+ goto exit;
+ }
wrqu->length = strlen(extra);
- return 0;
+
+exit:
+ kfree(input);
+ return ret;
}
static int rtw_mp_psd(struct net_device *dev,
--
1.9.1
Function rtw_mp_QueryDrv() dynamically allocates a temporary buffer that
is not freed in all error paths. Use a centralized exit path and make sure
that all memory is freed correctly. Detected by Coverity - CID 1077713.
Signed-off-by: Christian Engelmayer <[email protected]>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 1bd476d..8b1579b 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -7350,12 +7350,15 @@ static int rtw_mp_QueryDrv(struct net_device *dev,
char *input = kmalloc(wrqu->data.length, GFP_KERNEL);
u8 qAutoLoad = 1;
struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(padapter);
+ int ret = 0;
if (!input)
return -ENOMEM;
- if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length))
- return -EFAULT;
+ if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length)) {
+ ret = -EFAULT;
+ goto exit;
+ }
DBG_88E("%s:iwpriv in =%s\n", __func__, input);
qAutoLoad = strncmp(input, "autoload", 8); /* strncmp true is 0 */
@@ -7369,8 +7372,10 @@ static int rtw_mp_QueryDrv(struct net_device *dev,
sprintf(extra, "ok");
}
wrqu->data.length = strlen(extra) + 1;
+
+exit:
kfree(input);
- return 0;
+ return ret;
}
static int rtw_mp_set(struct net_device *dev,
--
1.9.1
Function rtw_mp_SetRFPath() dynamically allocates a temporary buffer that
is not freed in all error paths. Use a centralized exit path and make sure
that all memory is freed correctly. Detected by Coverity - CID 1077714.
Signed-off-by: Christian Engelmayer <[email protected]>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 8b1579b..ea5e1f8 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -7321,11 +7321,14 @@ static int rtw_mp_SetRFPath(struct net_device *dev,
struct adapter *padapter = rtw_netdev_priv(dev);
char *input = kmalloc(wrqu->data.length, GFP_KERNEL);
u8 bMain = 1, bTurnoff = 1;
+ int ret = 0;
if (!input)
return -ENOMEM;
- if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length))
- return -EFAULT;
+ if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length)) {
+ ret = -EFAULT;
+ goto exit;
+ }
DBG_88E("%s:iwpriv in =%s\n", __func__, input);
bMain = strncmp(input, "1", 2); /* strncmp true is 0 */
@@ -7338,8 +7341,10 @@ static int rtw_mp_SetRFPath(struct net_device *dev,
MP_PHY_SetRFPathSwitch(padapter, false);
DBG_88E("%s:PHY_SetRFPathSwitch = false\n", __func__);
}
+
+exit:
kfree(input);
- return 0;
+ return ret;
}
static int rtw_mp_QueryDrv(struct net_device *dev,
--
1.9.1
Function rtw_wx_set_enc_ext() dynamically allocates a temporary buffer that
is not freed in all error paths. Use a centralized exit path and make sure
that all memory is freed correctly. Detected by Coverity - CID 1077712.
Signed-off-by: Christian Engelmayer <[email protected]>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 45b47e2..1bd476d 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -2097,7 +2097,8 @@ static int rtw_wx_set_enc_ext(struct net_device *dev,
alg_name = "CCMP";
break;
default:
- return -1;
+ ret = -1;
+ goto exit;
}
strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
@@ -2124,6 +2125,7 @@ static int rtw_wx_set_enc_ext(struct net_device *dev,
ret = wpa_set_encryption(dev, param, param_len);
+exit:
kfree(param);
return ret;
}
--
1.9.1
On 04/28/2014 03:56 PM, Christian Engelmayer wrote:
> Function rtw_wx_read32() dynamically allocates a temporary buffer that is not
> freed in all error paths. Use a centralized exit path and make sure that all
> memory is freed correctly. Detected by Coverity - CID 1077711.
>
> Signed-off-by: Christian Engelmayer <[email protected]>
Acked-by: Larry Finger <[email protected]>
This Ack is valid for all 5 patches.
Larry
> ---
> drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index cf30a08..45b47e2 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -2154,6 +2154,7 @@ static int rtw_wx_read32(struct net_device *dev,
> u32 bytes;
> u8 *ptmp;
> int rv;
> + int ret = 0;
>
> padapter = (struct adapter *)rtw_netdev_priv(dev);
> p = &wrqu->data;
> @@ -2163,16 +2164,16 @@ static int rtw_wx_read32(struct net_device *dev,
> return -ENOMEM;
>
> if (copy_from_user(ptmp, p->pointer, len)) {
> - kfree(ptmp);
> - return -EFAULT;
> + ret = -EFAULT;
> + goto exit;
> }
>
> bytes = 0;
> addr = 0;
> rv = sscanf(ptmp, "%d,%x", &bytes, &addr);
> if (rv != 2) {
> - kfree(ptmp);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto exit;
> }
>
> switch (bytes) {
> @@ -2190,12 +2191,14 @@ static int rtw_wx_read32(struct net_device *dev,
> break;
> default:
> DBG_88E(KERN_INFO "%s: usage> read [bytes],[address(hex)]\n", __func__);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto exit;
> }
> DBG_88E(KERN_INFO "%s: addr = 0x%08X data =%s\n", __func__, addr, extra);
>
> +exit:
> kfree(ptmp);
> - return 0;
> + return ret;
> }
>
> static int rtw_wx_write32(struct net_device *dev,
>