* [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path
@ 2026-05-19 8:21 ZhaoJinming
2026-05-19 14:32 ` Ilpo Järvinen
2026-05-19 16:35 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path srinivas pandruvada
0 siblings, 2 replies; 14+ messages in thread
From: ZhaoJinming @ 2026-05-19 8:21 UTC (permalink / raw)
To: srinivas.pandruvada, hansg, ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, ZhaoJinming, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path 2026-05-19 8:21 [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming @ 2026-05-19 14:32 ` Ilpo Järvinen 2026-05-20 5:41 ` ZhaoJinming 2026-05-20 5:54 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-19 16:35 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path srinivas pandruvada 1 sibling, 2 replies; 14+ messages in thread From: Ilpo Järvinen @ 2026-05-19 14:32 UTC (permalink / raw) To: ZhaoJinming Cc: srinivas.pandruvada, Hans de Goede, platform-driver-x86, LKML, stable 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path 2026-05-19 14:32 ` Ilpo Järvinen @ 2026-05-20 5:41 ` ZhaoJinming 2026-05-20 5:41 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-20 5:41 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock " ZhaoJinming 2026-05-20 5:54 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 1 sibling, 2 replies; 14+ messages in thread From: ZhaoJinming @ 2026-05-20 5:41 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel Hi, Thank you for your kindness. I hope I got your point. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-20 5:41 ` ZhaoJinming @ 2026-05-20 5:41 ` ZhaoJinming 2026-05-20 10:37 ` Ilpo Järvinen 2026-05-20 5:41 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock " ZhaoJinming 1 sibling, 1 reply; 14+ messages in thread From: ZhaoJinming @ 2026-05-20 5:41 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, ZhaoJinming, stable 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-20 5:41 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming @ 2026-05-20 10:37 ` Ilpo Järvinen 2026-05-21 3:56 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming 0 siblings, 1 reply; 14+ messages in thread From: Ilpo Järvinen @ 2026-05-20 10:37 UTC (permalink / raw) To: ZhaoJinming Cc: srinivas.pandruvada, Hans de Goede, platform-driver-x86, LKML, stable 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path 2026-05-20 10:37 ` Ilpo Järvinen @ 2026-05-21 3:56 ` ZhaoJinming 2026-05-21 3:56 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-21 3:56 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex in mem_write() to guard ZhaoJinming 0 siblings, 2 replies; 14+ messages in thread From: ZhaoJinming @ 2026-05-21 3:56 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-21 3:56 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming @ 2026-05-21 3:56 ` ZhaoJinming 2026-05-21 12:58 ` Ilpo Järvinen 2026-05-21 3:56 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex in mem_write() to guard ZhaoJinming 1 sibling, 1 reply; 14+ messages in thread From: ZhaoJinming @ 2026-05-21 3:56 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, ZhaoJinming, stable 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-21 3:56 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming @ 2026-05-21 12:58 ` Ilpo Järvinen 2026-05-21 13:13 ` 赵金明 0 siblings, 1 reply; 14+ messages in thread From: Ilpo Järvinen @ 2026-05-21 12:58 UTC (permalink / raw) To: ZhaoJinming Cc: srinivas.pandruvada, Hans de Goede, platform-driver-x86, LKML, stable 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-21 12:58 ` Ilpo Järvinen @ 2026-05-21 13:13 ` 赵金明 0 siblings, 0 replies; 14+ messages in thread From: 赵金明 @ 2026-05-21 13:13 UTC (permalink / raw) To: ilpo.jarvinen Cc: srinivas.pandruvada, hansg, platform-driver-x86, linux-kernel, stable >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. > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] platform/x86/intel/tpmi: convert mutex in mem_write() to guard 2026-05-21 3:56 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming 2026-05-21 3:56 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming @ 2026-05-21 3:56 ` ZhaoJinming 1 sibling, 0 replies; 14+ messages in thread From: ZhaoJinming @ 2026-05-21 3:56 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, ZhaoJinming 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard 2026-05-20 5:41 ` ZhaoJinming 2026-05-20 5:41 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming @ 2026-05-20 5:41 ` ZhaoJinming 1 sibling, 0 replies; 14+ messages in thread From: ZhaoJinming @ 2026-05-20 5:41 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, ZhaoJinming 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() 2026-05-19 14:32 ` Ilpo Järvinen 2026-05-20 5:41 ` ZhaoJinming @ 2026-05-20 5:54 ` ZhaoJinming 2026-05-20 5:54 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard ZhaoJinming 1 sibling, 1 reply; 14+ messages in thread From: ZhaoJinming @ 2026-05-20 5:54 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, stable, ZhaoJinming 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard 2026-05-20 5:54 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming @ 2026-05-20 5:54 ` ZhaoJinming 0 siblings, 0 replies; 14+ messages in thread From: ZhaoJinming @ 2026-05-20 5:54 UTC (permalink / raw) To: ilpo.jarvinen, srinivas.pandruvada Cc: hansg, platform-driver-x86, linux-kernel, stable, ZhaoJinming 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path 2026-05-19 8:21 [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming 2026-05-19 14:32 ` Ilpo Järvinen @ 2026-05-19 16:35 ` srinivas pandruvada 1 sibling, 0 replies; 14+ messages in thread From: srinivas pandruvada @ 2026-05-19 16:35 UTC (permalink / raw) To: ZhaoJinming, hansg, ilpo.jarvinen Cc: platform-driver-x86, linux-kernel, stable 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; ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-05-21 13:13 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-05-19 8:21 [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming 2026-05-19 14:32 ` Ilpo Järvinen 2026-05-20 5:41 ` ZhaoJinming 2026-05-20 5:41 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-20 10:37 ` Ilpo Järvinen 2026-05-21 3:56 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path ZhaoJinming 2026-05-21 3:56 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-21 12:58 ` Ilpo Järvinen 2026-05-21 13:13 ` 赵金明 2026-05-21 3:56 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex in mem_write() to guard ZhaoJinming 2026-05-20 5:41 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock " ZhaoJinming 2026-05-20 5:54 ` [PATCH 1/2] platform/x86/intel/tpmi: use cleanup helpers in mem_write() ZhaoJinming 2026-05-20 5:54 ` [PATCH 2/2] platform/x86/intel/tpmi: convert mutex lock/unlock in mem_write() to guard ZhaoJinming 2026-05-19 16:35 ` [PATCH] platform/x86/intel/tpmi: Fix memory leak in mem_write() error path srinivas pandruvada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox