linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()
@ 2021-12-09  7:19 Jianglei Nie
  2021-12-09  7:56 ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Jianglei Nie @ 2021-12-09  7:19 UTC (permalink / raw)
  To: Larry.Finger, phil, gregkh, straube.linux, martin
  Cc: linux-staging, linux-kernel, Jianglei Nie

Line 5961 (#1) allocates a memory chunk for input by kmalloc().
Line 5966 (#2), line 5982 (#4) and line 5987 (#5) free the input
before the function returns while line 5979 (#3) forget to free it,
which will lead to a memory leak.

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

5953 static int rtw_mp_pwrtrk(struct net_device *dev,
5954 			struct iw_request_info *info,
5955 			struct iw_point *wrqu, char *extra)
5956 {
5961 	char	*input = kmalloc(wrqu->length, GFP_KERNEL);
	// #1: kmalloc space
5963 	if (!input)
5964 		return -ENOMEM;
5965 	if (copy_from_user(input, wrqu->pointer, wrqu->length)) {
5966 		kfree(input); // #2: kfree space
5967 		return -EFAULT;
5968	}

5973	if (strncmp(input, "stop", 4) == 0) {
5974		enable = 0;
5975		sprintf(extra, "mp tx power tracking stop");
5976	} else if (sscanf(input, "ther =%d", &thermal)) {
5977		ret = Hal_SetThermalMeter(padapter, (u8)thermal);
5978		if (ret == _FAIL)
5979			return -EPERM; // #3: missing kfree
5980		sprintf(extra, "mp tx power tracking start,
			target value =%d ok ", thermal);
5981	} else {
5982		kfree(input); // #4: kfree space
5983		return -EINVAL;
5984	}

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

5993	return 0;
5994 }

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
 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 1fd375076001..8f9e0f12c51f 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -5975,8 +5975,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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()
  2021-12-09  7:19 [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk() Jianglei Nie
@ 2021-12-09  7:56 ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-12-09  7:56 UTC (permalink / raw)
  To: Jianglei Nie
  Cc: Larry.Finger, phil, straube.linux, martin, linux-staging, linux-kernel

On Thu, Dec 09, 2021 at 03:19:05PM +0800, Jianglei Nie wrote:
> Line 5961 (#1) allocates a memory chunk for input by kmalloc().
> Line 5966 (#2), line 5982 (#4) and line 5987 (#5) free the input
> before the function returns while line 5979 (#3) forget to free it,
> which will lead to a memory leak.
> 
> We should kfree() input in line 5979 (#3).
> 
> 5953 static int rtw_mp_pwrtrk(struct net_device *dev,
> 5954 			struct iw_request_info *info,
> 5955 			struct iw_point *wrqu, char *extra)
> 5956 {
> 5961 	char	*input = kmalloc(wrqu->length, GFP_KERNEL);
> 	// #1: kmalloc space
> 5963 	if (!input)
> 5964 		return -ENOMEM;
> 5965 	if (copy_from_user(input, wrqu->pointer, wrqu->length)) {
> 5966 		kfree(input); // #2: kfree space
> 5967 		return -EFAULT;
> 5968	}
> 
> 5973	if (strncmp(input, "stop", 4) == 0) {
> 5974		enable = 0;
> 5975		sprintf(extra, "mp tx power tracking stop");
> 5976	} else if (sscanf(input, "ther =%d", &thermal)) {
> 5977		ret = Hal_SetThermalMeter(padapter, (u8)thermal);
> 5978		if (ret == _FAIL)
> 5979			return -EPERM; // #3: missing kfree
> 5980		sprintf(extra, "mp tx power tracking start,
> 			target value =%d ok ", thermal);
> 5981	} else {
> 5982		kfree(input); // #4: kfree space
> 5983		return -EINVAL;
> 5984	}
> 
> 5987	kfree(input); // #5: kfree space
> 
> 5993	return 0;
> 5994 }
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>  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 1fd375076001..8f9e0f12c51f 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -5975,8 +5975,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);

What kernel tree and version did you make this patch against?  I do not
even see this function in Linus's tree, nor in my staging-next
development branch.

confused,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()
  2021-12-09 13:25 Jianglei Nie
  2021-12-09 14:28 ` Dan Carpenter
@ 2021-12-09 15:08 ` Greg KH
  1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-12-09 15:08 UTC (permalink / raw)
  To: Jianglei Nie
  Cc: Larry.Finger, phil, straube.linux, martin, linux-staging, linux-kernel

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 <niejianglei2021@163.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()
  2021-12-09 13:25 Jianglei Nie
@ 2021-12-09 14:28 ` Dan Carpenter
  2021-12-09 15:08 ` Greg KH
  1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-12-09 14:28 UTC (permalink / raw)
  To: Jianglei Nie
  Cc: Larry.Finger, phil, gregkh, straube.linux, martin, linux-staging,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk()
@ 2021-12-09 13:25 Jianglei Nie
  2021-12-09 14:28 ` Dan Carpenter
  2021-12-09 15:08 ` Greg KH
  0 siblings, 2 replies; 5+ messages in thread
From: Jianglei Nie @ 2021-12-09 13:25 UTC (permalink / raw)
  To: Larry.Finger, phil, gregkh, straube.linux, martin
  Cc: linux-staging, linux-kernel, Jianglei Nie

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 <niejianglei2021@163.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-09 15:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09  7:19 [PATCH] staging: r8188eu: fix a memory leak in rtw_mp_pwrtrk() Jianglei Nie
2021-12-09  7:56 ` Greg KH
2021-12-09 13:25 Jianglei Nie
2021-12-09 14:28 ` Dan Carpenter
2021-12-09 15:08 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).