2021-12-09 13:25:57

by Jianglei Nie

[permalink] [raw]
Subject: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()

Line 5968 (#1) allocates a memory chunk for input by kmalloc().
Line 5973 (#2), line 5989 (#4) and line 5994 (#5) free the input
before the function returns while line 5986 (#3) forget to free it,
which will lead to a memory leak. This bug influences all stable
versions from 5.15.1 to 5.15.7.

We should kfree() input in line 5986 (#3).

5960 static int rtw_mp_pwrtrk(struct net_device *dev,
5961 struct iw_request_info *info,
5962 struct iw_point *wrqu, char *extra)
5963 {
5968 char *input = kmalloc(wrqu->length, GFP_KERNEL);
// #1: kmalloc space
5970 if (!input)
5971 return -ENOMEM;
5972 if (copy_from_user(input, wrqu->pointer, wrqu->length)) {
5973 kfree(input); // #2: kfree space
5974 return -EFAULT;
5975 }

5980 if (strncmp(input, "stop", 4) == 0) {
5981 enable = 0;
5982 sprintf(extra, "mp tx power tracking stop");
5983 } else if (sscanf(input, "ther =%d", &thermal)) {
5984 ret = Hal_SetThermalMeter(padapter, (u8)thermal);
5985 if (ret == _FAIL)
5986 return -EPERM; // #3: missing kfree
5987 sprintf(extra, "mp tx power tracking start,
target value =%d ok ", thermal);
5988 } else {
5989 kfree(input); // #4: kfree space
5990 return -EINVAL;
5991 }

5994 kfree(input); // #5: kfree space

6000 return 0;
6001 }

Signed-off-by: Jianglei Nie <[email protected]>
---
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 0eccce57c63a..906a57eae1af 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -5982,8 +5982,10 @@ 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)
+ if (ret == _FAIL) {
+ kfree(input);
return -EPERM;
+ }
sprintf(extra, "mp tx power tracking start, target value =%d ok ", thermal);
} else {
kfree(input);
--
2.25.1



2021-12-09 13:26:05

by Jianglei Nie

[permalink] [raw]
Subject: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_QueryDrv()

Line 6191 (#1) allocates a memory chunk for input by kmalloc().
Line 6213 (#3) frees the input before the function returns while
line 6199 (#2) forget to free it, which will lead to a memory leak.
This bug influences all stable versions from 5.15.1 to 5.15.7.

We should kfree() input in line 6199 (#2).

6186 static int rtw_mp_QueryDrv(struct net_device *dev,
6187 struct iw_request_info *info,
6188 union iwreq_data *wrqu, char *extra)
6189 {
6191 char *input = kmalloc(wrqu->data.length, GFP_KERNEL);
// #1: kmalloc space

6195 if (!input)
6196 return -ENOMEM;

6198 if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length))
6199 return -EFAULT; // #2: missing kfree

6213 kfree(input); // #3: kfree space
6214 return 0;
6215 }

Signed-off-by: Jianglei Nie <[email protected]>
---
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 906a57eae1af..edc660f15436 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -6195,8 +6195,11 @@ static int rtw_mp_QueryDrv(struct net_device *dev,
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)) {
+ kfree(input);
+ return -EFAULT;
+ }
+
DBG_88E("%s:iwpriv in =%s\n", __func__, input);

qAutoLoad = strncmp(input, "autoload", 8); /* strncmp true is 0 */
--
2.25.1


2021-12-09 14:29:24

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()

On Thu, Dec 09, 2021 at 09:25:15PM +0800, Jianglei Nie wrote:
> Line 5968 (#1) allocates a memory chunk for input by kmalloc().
> Line 5973 (#2), line 5989 (#4) and line 5994 (#5) free the input
> before the function returns while line 5986 (#3) forget to free it,
> which will lead to a memory leak. This bug influences all stable
> versions from 5.15.1 to 5.15.7.
>

This code was all deleted in September. Please work against
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next
or linux-next.

regards,
dan carpenter


2021-12-09 14:31:30

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_QueryDrv()

This one is obsolete as well.

regards,
dan carpenter


2021-12-09 15:08:38

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()

On Thu, Dec 09, 2021 at 09:25:15PM +0800, Jianglei Nie wrote:
> Line 5968 (#1) allocates a memory chunk for input by kmalloc().
> Line 5973 (#2), line 5989 (#4) and line 5994 (#5) free the input
> before the function returns while line 5986 (#3) forget to free it,
> which will lead to a memory leak. This bug influences all stable
> versions from 5.15.1 to 5.15.7.
>
> We should kfree() input in line 5986 (#3).
>
> 5960 static int rtw_mp_pwrtrk(struct net_device *dev,
> 5961 struct iw_request_info *info,
> 5962 struct iw_point *wrqu, char *extra)
> 5963 {
> 5968 char *input = kmalloc(wrqu->length, GFP_KERNEL);
> // #1: kmalloc space
> 5970 if (!input)
> 5971 return -ENOMEM;
> 5972 if (copy_from_user(input, wrqu->pointer, wrqu->length)) {
> 5973 kfree(input); // #2: kfree space
> 5974 return -EFAULT;
> 5975 }
>
> 5980 if (strncmp(input, "stop", 4) == 0) {
> 5981 enable = 0;
> 5982 sprintf(extra, "mp tx power tracking stop");
> 5983 } else if (sscanf(input, "ther =%d", &thermal)) {
> 5984 ret = Hal_SetThermalMeter(padapter, (u8)thermal);
> 5985 if (ret == _FAIL)
> 5986 return -EPERM; // #3: missing kfree
> 5987 sprintf(extra, "mp tx power tracking start,
> target value =%d ok ", thermal);
> 5988 } else {
> 5989 kfree(input); // #4: kfree space
> 5990 return -EINVAL;
> 5991 }
>
> 5994 kfree(input); // #5: kfree space
>
> 6000 return 0;
> 6001 }
>
> Signed-off-by: Jianglei Nie <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 0eccce57c63a..906a57eae1af 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -5982,8 +5982,10 @@ 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)
> + if (ret == _FAIL) {
> + kfree(input);
> return -EPERM;
> + }
> sprintf(extra, "mp tx power tracking start, target value =%d ok ", thermal);
> } else {
> kfree(input);
> --
> 2.25.1
>
>

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2021-12-09 15:08:51

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_QueryDrv()

On Thu, Dec 09, 2021 at 09:25:16PM +0800, Jianglei Nie wrote:
> Line 6191 (#1) allocates a memory chunk for input by kmalloc().
> Line 6213 (#3) frees the input before the function returns while
> line 6199 (#2) forget to free it, which will lead to a memory leak.
> This bug influences all stable versions from 5.15.1 to 5.15.7.
>
> We should kfree() input in line 6199 (#2).
>
> 6186 static int rtw_mp_QueryDrv(struct net_device *dev,
> 6187 struct iw_request_info *info,
> 6188 union iwreq_data *wrqu, char *extra)
> 6189 {
> 6191 char *input = kmalloc(wrqu->data.length, GFP_KERNEL);
> // #1: kmalloc space
>
> 6195 if (!input)
> 6196 return -ENOMEM;
>
> 6198 if (copy_from_user(input, wrqu->data.pointer, wrqu->data.length))
> 6199 return -EFAULT; // #2: missing kfree
>
> 6213 kfree(input); // #3: kfree space
> 6214 return 0;
> 6215 }
>
> Signed-off-by: Jianglei Nie <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/ioctl_linux.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 906a57eae1af..edc660f15436 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -6195,8 +6195,11 @@ static int rtw_mp_QueryDrv(struct net_device *dev,
> 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)) {
> + kfree(input);
> + return -EFAULT;
> + }
> +
> DBG_88E("%s:iwpriv in =%s\n", __func__, input);
>
> qAutoLoad = strncmp(input, "autoload", 8); /* strncmp true is 0 */
> --
> 2.25.1
>
>

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot