[PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path

ZhaoJinming posted 1 patch 5 days, 18 hours ago
drivers/platform/x86/intel/vsec_tpmi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
Posted by ZhaoJinming 5 days, 18 hours ago
In mem_write(), when the IS_ALIGNED() check fails, the function returns
-EINVAL directly without freeing the 'array' allocated by
parse_int_array_user(). This causes a memory leak.

Other error paths in the same function correctly use 'goto exit_write'
to free the array before returning. Fix this inconsistency by using
the same pattern for the alignment check.

Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index 16fd7aa41f20..2a428bfcb209 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -495,8 +495,10 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	addr = array[2];
 	value = array[3];
 
-	if (!IS_ALIGNED(addr, sizeof(u32)))
-		return -EINVAL;
+	if (!IS_ALIGNED(addr, sizeof(u32))) {
+		ret = -EINVAL;
+		goto exit_write;
+	}
 
 	if (punit >= pfs->pfs_header.num_entries) {
 		ret = -EINVAL;
-- 
2.20.1
Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
Posted by srinivas pandruvada 5 days, 10 hours ago
On Tue, 2026-05-19 at 16:21 +0800, ZhaoJinming wrote:
> In mem_write(), when the IS_ALIGNED() check fails, the function
> returns
> -EINVAL directly without freeing the 'array' allocated by
> parse_int_array_user(). This causes a memory leak.
> 
> Other error paths in the same function correctly use 'goto
> exit_write'
> to free the array before returning. Fix this inconsistency by using
> the same pattern for the alignment check.
> 

Thanks. I see Ilpo suggested using cleanup.h. Let me know if you have
issue in doing that.

-Srinivas

> Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned
> address for debugfs mem write")
> Cc: stable@vger.kernel.org
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/platform/x86/intel/vsec_tpmi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vsec_tpmi.c
> b/drivers/platform/x86/intel/vsec_tpmi.c
> index 16fd7aa41f20..2a428bfcb209 100644
> --- a/drivers/platform/x86/intel/vsec_tpmi.c
> +++ b/drivers/platform/x86/intel/vsec_tpmi.c
> @@ -495,8 +495,10 @@ static ssize_t mem_write(struct file *file,
> const char __user *userbuf, size_t l
>  	addr = array[2];
>  	value = array[3];
>  
> -	if (!IS_ALIGNED(addr, sizeof(u32)))
> -		return -EINVAL;
> +	if (!IS_ALIGNED(addr, sizeof(u32))) {
> +		ret = -EINVAL;
> +		goto exit_write;
> +	}
>  
>  	if (punit >= pfs->pfs_header.num_entries) {
>  		ret = -EINVAL;
Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
Posted by Ilpo Järvinen 5 days, 12 hours ago
On Tue, 19 May 2026, ZhaoJinming wrote:

> In mem_write(), when the IS_ALIGNED() check fails, the function returns
> -EINVAL directly without freeing the 'array' allocated by
> parse_int_array_user(). This causes a memory leak.
> 
> Other error paths in the same function correctly use 'goto exit_write'
> to free the array before returning. Fix this inconsistency by using
> the same pattern for the alignment check.
> 
> Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
> Cc: stable@vger.kernel.org
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/platform/x86/intel/vsec_tpmi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
> index 16fd7aa41f20..2a428bfcb209 100644
> --- a/drivers/platform/x86/intel/vsec_tpmi.c
> +++ b/drivers/platform/x86/intel/vsec_tpmi.c
> @@ -495,8 +495,10 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	addr = array[2];
>  	value = array[3];
>  
> -	if (!IS_ALIGNED(addr, sizeof(u32)))
> -		return -EINVAL;
> +	if (!IS_ALIGNED(addr, sizeof(u32))) {
> +		ret = -EINVAL;
> +		goto exit_write;
> +	}
>  
>  	if (punit >= pfs->pfs_header.num_entries) {
>  		ret = -EINVAL;

Hi,

Thanks for finding this problem. This function looks a prime candidate for 
cleanup.h conversion.

Please do this with a 2 patch series. The first patch converts kfree() to 
__free() and moves array declaration next to parse_int_array_user() (as 
instructed by the long comment in cleanup.h) and has fixes tag. The second 
patch should converyt the mutex lock/unlock to guard and doesn't need 
fixes tag. This way, we get rid of all gotos here.

-- 
 i.
Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
Posted by ZhaoJinming 4 days, 21 hours ago
Hi,

Thank you for your kindness. I hope I got your point.


[PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by ZhaoJinming 4 days, 21 hours ago
In mem_write(), the temporary array returned by parse_int_array_user() must be released on all error paths. Convert the array variable to use cleanup.h scope-based cleanup so it is freed automatically on return.

This also moves the array declaration next to parse_int_array_user() as required by cleanup.h usage guidelines.

Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index 16fd7aa41f20..e7bc3474c7aa 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -51,6 +51,7 @@
 #include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/cleanup.h>
 #include <linux/intel_tpmi.h>
 #include <linux/intel_vsec.h>
 #include <linux/io.h>
@@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	struct seq_file *m = file->private_data;
 	struct intel_tpmi_pm_feature *pfs = m->private;
 	u32 addr, value, punit, size;
-	u32 num_elems, *array;
+	u32 num_elems;
 	void __iomem *mem;
 	int ret;
 
@@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!size)
 		return -EIO;
 
+	u32 *array __free(kfree) = NULL;
 	ret = parse_int_array_user(userbuf, len, (int **)&array);
 	if (ret < 0)
 		return ret;
 
 	num_elems = *array;
-	if (num_elems != 3) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (num_elems != 3)
+		return -EINVAL;
 
 	punit = array[1];
 	addr = array[2];
@@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!IS_ALIGNED(addr, sizeof(u32)))
 		return -EINVAL;
 
-	if (punit >= pfs->pfs_header.num_entries) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (punit >= pfs->pfs_header.num_entries)
+		return -EINVAL;
 
-	if (addr >= size) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (addr >= size)
+		return -EINVAL;
 
 	mutex_lock(&tpmi_dev_lock);
 
@@ -522,12 +518,8 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 
 	ret = len;
 
-unlock_mem_write:
 	mutex_unlock(&tpmi_dev_lock);
 
-exit_write:
-	kfree(array);
-
 	return ret;
 }
 
-- 
2.20.1
Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by Ilpo Järvinen 4 days, 16 hours ago
On Wed, 20 May 2026, ZhaoJinming wrote:

> In mem_write(), the temporary array returned by parse_int_array_user() must be released on all error paths. Convert the array variable to use cleanup.h scope-based cleanup so it is freed automatically on return.

Not only on "error paths" but also when no error occurs.

> This also moves the array declaration next to parse_int_array_user() as required by cleanup.h usage guidelines.

Please fold any text paragraphs so they don't exceed 72 characters per 
row.

> Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
> Cc: stable@vger.kernel.org
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/platform/x86/intel/vsec_tpmi.c | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
> index 16fd7aa41f20..e7bc3474c7aa 100644
> --- a/drivers/platform/x86/intel/vsec_tpmi.c
> +++ b/drivers/platform/x86/intel/vsec_tpmi.c
> @@ -51,6 +51,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
> +#include <linux/cleanup.h>
>  #include <linux/intel_tpmi.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/io.h>
> @@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	struct seq_file *m = file->private_data;
>  	struct intel_tpmi_pm_feature *pfs = m->private;
>  	u32 addr, value, punit, size;
> -	u32 num_elems, *array;
> +	u32 num_elems;
>  	void __iomem *mem;
>  	int ret;
>  
> @@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	if (!size)
>  		return -EIO;
>  
> +	u32 *array __free(kfree) = NULL;
>  	ret = parse_int_array_user(userbuf, len, (int **)&array);
>  	if (ret < 0)
>  		return ret;
>  
>  	num_elems = *array;
> -	if (num_elems != 3) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (num_elems != 3)
> +		return -EINVAL;
>  
>  	punit = array[1];
>  	addr = array[2];
> @@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	if (!IS_ALIGNED(addr, sizeof(u32)))
>  		return -EINVAL;
>  
> -	if (punit >= pfs->pfs_header.num_entries) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (punit >= pfs->pfs_header.num_entries)
> +		return -EINVAL;
>  
> -	if (addr >= size) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (addr >= size)
> +		return -EINVAL;
>  
>  	mutex_lock(&tpmi_dev_lock);
>  
> @@ -522,12 +518,8 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  
>  	ret = len;
>  
> -unlock_mem_write:

The last goto is only removed in the second patch so this will cause a 
build failure mid-series which would be a problem when using git bisect.

Other than that, this change looked okay.

>  	mutex_unlock(&tpmi_dev_lock);
>  
> -exit_write:
> -	kfree(array);
> -
>  	return ret;
>  }
>  
> 

-- 
 i.
Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
Posted by ZhaoJinming 3 days, 23 hours ago
Updated two patches, moved remove-goto-tag to the second patch.
Enabled CONFIG_INTEL_TPMI, build -> patch 001 -> build -> patch 002 -> build. 
Build kernel successfully.
[PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by ZhaoJinming 3 days, 23 hours ago
In mem_write(), the temporary array returned by
parse_int_array_user() must be released on all exit paths.
Convert the array variable to use cleanup.h scope-based
cleanup so it is freed automatically on return.

This also moves the array declaration next to
parse_int_array_user() as required by cleanup.h usage
guidelines.

Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index 16fd7aa41f20..88f14d0ad410 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -50,6 +50,7 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/bitfield.h>
 #include <linux/debugfs.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/intel_tpmi.h>
 #include <linux/intel_vsec.h>
@@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	struct seq_file *m = file->private_data;
 	struct intel_tpmi_pm_feature *pfs = m->private;
 	u32 addr, value, punit, size;
-	u32 num_elems, *array;
+	u32 num_elems;
 	void __iomem *mem;
 	int ret;
 
@@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!size)
 		return -EIO;
 
+	u32 *array __free(kfree) = NULL;
 	ret = parse_int_array_user(userbuf, len, (int **)&array);
 	if (ret < 0)
 		return ret;
 
 	num_elems = *array;
-	if (num_elems != 3) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (num_elems != 3)
+		return -EINVAL;
 
 	punit = array[1];
 	addr = array[2];
@@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!IS_ALIGNED(addr, sizeof(u32)))
 		return -EINVAL;
 
-	if (punit >= pfs->pfs_header.num_entries) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (punit >= pfs->pfs_header.num_entries)
+		return -EINVAL;
 
-	if (addr >= size) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (addr >= size)
+		return -EINVAL;
 
 	mutex_lock(&tpmi_dev_lock);
 
@@ -525,9 +521,6 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 unlock_mem_write:
 	mutex_unlock(&tpmi_dev_lock);
 
-exit_write:
-	kfree(array);
-
 	return ret;
 }
 
-- 
2.20.1
Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by Ilpo Järvinen 3 days, 13 hours ago
On Thu, 21 May 2026, ZhaoJinming wrote:

> In mem_write(), the temporary array returned by
> parse_int_array_user() must be released on all exit paths.
> Convert the array variable to use cleanup.h scope-based
> cleanup so it is freed automatically on return.
> 
> This also moves the array declaration next to
> parse_int_array_user() as required by cleanup.h usage
> guidelines.

Now you made these much shorter than 72 chars. :-(

> Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
> Cc: stable@vger.kernel.org
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---
>  drivers/platform/x86/intel/vsec_tpmi.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
> index 16fd7aa41f20..88f14d0ad410 100644
> --- a/drivers/platform/x86/intel/vsec_tpmi.c
> +++ b/drivers/platform/x86/intel/vsec_tpmi.c
> @@ -50,6 +50,7 @@
>  #include <linux/auxiliary_bus.h>
>  #include <linux/bitfield.h>
>  #include <linux/debugfs.h>
> +#include <linux/cleanup.h>
>  #include <linux/delay.h>
>  #include <linux/intel_tpmi.h>
>  #include <linux/intel_vsec.h>
> @@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	struct seq_file *m = file->private_data;
>  	struct intel_tpmi_pm_feature *pfs = m->private;
>  	u32 addr, value, punit, size;
> -	u32 num_elems, *array;
> +	u32 num_elems;
>  	void __iomem *mem;
>  	int ret;
>  
> @@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	if (!size)
>  		return -EIO;
>  
> +	u32 *array __free(kfree) = NULL;
>  	ret = parse_int_array_user(userbuf, len, (int **)&array);
>  	if (ret < 0)
>  		return ret;
>  
>  	num_elems = *array;
> -	if (num_elems != 3) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (num_elems != 3)
> +		return -EINVAL;
>  
>  	punit = array[1];
>  	addr = array[2];
> @@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  	if (!IS_ALIGNED(addr, sizeof(u32)))
>  		return -EINVAL;
>  
> -	if (punit >= pfs->pfs_header.num_entries) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (punit >= pfs->pfs_header.num_entries)
> +		return -EINVAL;
>  
> -	if (addr >= size) {
> -		ret = -EINVAL;
> -		goto exit_write;
> -	}
> +	if (addr >= size)
> +		return -EINVAL;
>  
>  	mutex_lock(&tpmi_dev_lock);
>  
> @@ -525,9 +521,6 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
>  unlock_mem_write:
>  	mutex_unlock(&tpmi_dev_lock);
>  
> -exit_write:
> -	kfree(array);
> -
>  	return ret;
>  }
>  
> 

The code change looks okay now.

BUT, please send the next version properly versioned (it "v4" or so in 
it's subject, I think) and in a fresh thread. As is, b4 gets confused 
which patches are the latest version so I cannot apply these with my 
maintainer tools.

-- 
 i.
Re: Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by 赵金明 3 days, 13 hours ago



>On Thu, 21 May 2026, ZhaoJinming wrote:



>



>> In mem_write(), the temporary array returned by



>> parse_int_array_user() must be released on all exit paths.



>> Convert the array variable to use cleanup.h scope-based



>> cleanup so it is freed automatically on return.



>> 



>> This also moves the array declaration next to



>> parse_int_array_user() as required by cleanup.h usage



>> guidelines.



>



>Now you made these much shorter than 72 chars. :-(



>



>> Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")



>> Cc: stable@vger.kernel.org



>> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>



>> ---



>>? drivers/platform/x86/intel/vsec_tpmi.c | 25 +++++++++----------------



>>? 1 file changed, 9 insertions(+), 16 deletions(-)



>> 



>> diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c



>> index 16fd7aa41f20..88f14d0ad410 100644



>> --- a/drivers/platform/x86/intel/vsec_tpmi.c



>> +++ b/drivers/platform/x86/intel/vsec_tpmi.c



>> @@ -50,6 +50,7 @@



>>? #include <linux/auxiliary_bus.h>



>>? #include <linux/bitfield.h>



>>? #include <linux/debugfs.h>



>> +#include <linux/cleanup.h>



>>? #include <linux/delay.h>



>>? #include <linux/intel_tpmi.h>



>>? #include <linux/intel_vsec.h>



>> @@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l



>>? 	struct seq_file *m = file->private_data;



>>? 	struct intel_tpmi_pm_feature *pfs = m->private;



>>? 	u32 addr, value, punit, size;



>> -	u32 num_elems, *array;



>> +	u32 num_elems;



>>? 	void __iomem *mem;



>>? 	int ret;



>>? 



>> @@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l



>>? 	if (!size)



>>? 		return -EIO;



>>? 



>> +	u32 *array __free(kfree) = NULL;



>>? 	ret = parse_int_array_user(userbuf, len, (int **)&array);



>>? 	if (ret < 0)



>>? 		return ret;



>>? 



>>? 	num_elems = *array;



>> -	if (num_elems != 3) {



>> -		ret = -EINVAL;



>> -		goto exit_write;



>> -	}



>> +	if (num_elems != 3)



>> +		return -EINVAL;



>>? 



>>? 	punit = array[1];



>>? 	addr = array[2];



>> @@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l



>>? 	if (!IS_ALIGNED(addr, sizeof(u32)))



>>? 		return -EINVAL;



>>? 



>> -	if (punit >= pfs->pfs_header.num_entries) {



>> -		ret = -EINVAL;



>> -		goto exit_write;



>> -	}



>> +	if (punit >= pfs->pfs_header.num_entries)



>> +		return -EINVAL;



>>? 



>> -	if (addr >= size) {



>> -		ret = -EINVAL;



>> -		goto exit_write;



>> -	}



>> +	if (addr >= size)



>> +		return -EINVAL;



>>? 



>>? 	mutex_lock(&tpmi_dev_lock);



>>? 



>> @@ -525,9 +521,6 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l



>>? unlock_mem_write:



>>? 	mutex_unlock(&tpmi_dev_lock);



>>? 



>> -exit_write:



>> -	kfree(array);



>> -



>>? 	return ret;



>>? }



>>? 



>> 



>



>The code change looks okay now.



>



>BUT, please send the next version properly versioned (it "v4" or so in 



>it's subject, I think) and in a fresh thread. As is, b4 gets confused 



>which patches are the latest version so I cannot apply these with my 



>maintainer tools.



>

Thanks for your reply.
I've sent the v4 patches in a new thread.


>-- 



> i.



>



>


[PATCH 2/2] platform/x86/intel/tpmi: convert mutex in mem_write() to guard
Posted by ZhaoJinming 3 days, 23 hours ago
Convert the explicit mutex_lock/mutex_unlock pair in
mem_write() into a cleanup.h guard(mutex)() scope-based
lock acquisition. This removes the remaining goto-based
cleanup path and keeps the lock held until the end of the
mem_write() scope.

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index 88f14d0ad410..9bfd05eaa002 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -504,24 +504,17 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (addr >= size)
 		return -EINVAL;
 
-	mutex_lock(&tpmi_dev_lock);
+	guard(mutex)(&tpmi_dev_lock);
 
 	mem = ioremap(pfs->vsec_offset + punit * size, size);
-	if (!mem) {
-		ret = -ENOMEM;
-		goto unlock_mem_write;
-	}
+	if (!mem)
+		return -ENOMEM;
 
 	writel(value, mem + addr);
 
 	iounmap(mem);
 
-	ret = len;
-
-unlock_mem_write:
-	mutex_unlock(&tpmi_dev_lock);
-
-	return ret;
+	return len;
 }
 
 static int mem_write_show(struct seq_file *s, void *unused)
-- 
2.20.1
[PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard
Posted by ZhaoJinming 4 days, 21 hours ago
Convert the explicit mutex_lock/mutex_unlock pair in mem_write() into a cleanup.h guard(mutex)() scope-based lock acquisition. This removes the remaining goto-based cleanup path and keeps the lock held until the end of the mem_write() scope.

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index e7bc3474c7aa..72b78b505e03 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -504,23 +504,17 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (addr >= size)
 		return -EINVAL;
 
-	mutex_lock(&tpmi_dev_lock);
+	guard(mutex)(&tpmi_dev_lock);
 
 	mem = ioremap(pfs->vsec_offset + punit * size, size);
-	if (!mem) {
-		ret = -ENOMEM;
-		goto unlock_mem_write;
-	}
+	if (!mem)
+		return -ENOMEM;
 
 	writel(value, mem + addr);
 
 	iounmap(mem);
 
-	ret = len;
-
-	mutex_unlock(&tpmi_dev_lock);
-
-	return ret;
+	return len;
 }
 
 static int mem_write_show(struct seq_file *s, void *unused)
-- 
2.20.1
[PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write()
Posted by ZhaoJinming 4 days, 21 hours ago
In mem_write(), the temporary array returned by parse_int_array_user() must be released on all error paths. Convert the array variable to use cleanup.h scope-based cleanup so it is freed automatically on return.

This also moves the array declaration next to parse_int_array_user() as required by cleanup.h usage guidelines.

Fixes: 8e0a2fc68ec3 ("platform/x86/intel/tpmi: Use 32 bit aligned address for debugfs mem write")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index 16fd7aa41f20..e7bc3474c7aa 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -51,6 +51,7 @@
 #include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/cleanup.h>
 #include <linux/intel_tpmi.h>
 #include <linux/intel_vsec.h>
 #include <linux/io.h>
@@ -473,7 +474,7 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	struct seq_file *m = file->private_data;
 	struct intel_tpmi_pm_feature *pfs = m->private;
 	u32 addr, value, punit, size;
-	u32 num_elems, *array;
+	u32 num_elems;
 	void __iomem *mem;
 	int ret;
 
@@ -481,15 +482,14 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!size)
 		return -EIO;
 
+	u32 *array __free(kfree) = NULL;
 	ret = parse_int_array_user(userbuf, len, (int **)&array);
 	if (ret < 0)
 		return ret;
 
 	num_elems = *array;
-	if (num_elems != 3) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (num_elems != 3)
+		return -EINVAL;
 
 	punit = array[1];
 	addr = array[2];
@@ -498,15 +498,11 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (!IS_ALIGNED(addr, sizeof(u32)))
 		return -EINVAL;
 
-	if (punit >= pfs->pfs_header.num_entries) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (punit >= pfs->pfs_header.num_entries)
+		return -EINVAL;
 
-	if (addr >= size) {
-		ret = -EINVAL;
-		goto exit_write;
-	}
+	if (addr >= size)
+		return -EINVAL;
 
 	mutex_lock(&tpmi_dev_lock);
 
@@ -522,12 +518,8 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 
 	ret = len;
 
-unlock_mem_write:
 	mutex_unlock(&tpmi_dev_lock);
 
-exit_write:
-	kfree(array);
-
 	return ret;
 }
 
-- 
2.20.1
[PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard
Posted by ZhaoJinming 4 days, 21 hours ago
Convert the explicit mutex_lock/mutex_unlock pair in mem_write() into a cleanup.h guard(mutex)() scope-based lock acquisition. This removes the remaining goto-based cleanup path and keeps the lock held until the end of the mem_write() scope.

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/platform/x86/intel/vsec_tpmi.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel/vsec_tpmi.c b/drivers/platform/x86/intel/vsec_tpmi.c
index e7bc3474c7aa..72b78b505e03 100644
--- a/drivers/platform/x86/intel/vsec_tpmi.c
+++ b/drivers/platform/x86/intel/vsec_tpmi.c
@@ -504,23 +504,17 @@ static ssize_t mem_write(struct file *file, const char __user *userbuf, size_t l
 	if (addr >= size)
 		return -EINVAL;
 
-	mutex_lock(&tpmi_dev_lock);
+	guard(mutex)(&tpmi_dev_lock);
 
 	mem = ioremap(pfs->vsec_offset + punit * size, size);
-	if (!mem) {
-		ret = -ENOMEM;
-		goto unlock_mem_write;
-	}
+	if (!mem)
+		return -ENOMEM;
 
 	writel(value, mem + addr);
 
 	iounmap(mem);
 
-	ret = len;
-
-	mutex_unlock(&tpmi_dev_lock);
-
-	return ret;
+	return len;
 }
 
 static int mem_write_show(struct seq_file *s, void *unused)
-- 
2.20.1