* [PATCH] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
@ 2024-01-23 1:11 Cong Liu
2024-01-23 14:37 ` Shyam Sundar S K
0 siblings, 1 reply; 7+ messages in thread
From: Cong Liu @ 2024-01-23 1:11 UTC (permalink / raw)
To: Shyam Sundar S K, Hans de Goede, Ilpo Järvinen
Cc: Cong Liu, platform-driver-x86, linux-kernel
amd_pmf_get_pb_data() will allocate memory for the policy buffer,
but does not free it if copy_from_user() fails. This leads to a memory
leak.
Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
drivers/platform/x86/amd/pmf/tee-if.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 502ce93d5cdd..f8c0177afb0d 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -298,8 +298,10 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
if (!new_policy_buf)
return -ENOMEM;
- if (copy_from_user(new_policy_buf, buf, length))
+ if (copy_from_user(new_policy_buf, buf, length)) {
+ kfree(new_policy_buf);
return -EFAULT;
+ }
kfree(dev->policy_buf);
dev->policy_buf = new_policy_buf;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-23 1:11 [PATCH] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data() Cong Liu
@ 2024-01-23 14:37 ` Shyam Sundar S K
2024-01-24 1:29 ` [PATCH v2] " Cong Liu
0 siblings, 1 reply; 7+ messages in thread
From: Shyam Sundar S K @ 2024-01-23 14:37 UTC (permalink / raw)
To: Cong Liu, Hans de Goede, Ilpo Järvinen
Cc: platform-driver-x86, linux-kernel
Hi,
On 1/23/2024 06:41, Cong Liu wrote:
> amd_pmf_get_pb_data() will allocate memory for the policy buffer,
> but does not free it if copy_from_user() fails. This leads to a memory
> leak.
Thank you for the fix and looks good to me (just a valid Fixes tag is
missing.)
Fixes: 10817f28e533 ("platform/x86/amd/pmf: Add capability to sideload
of policy binary")
Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Thanks,
Shyam
>
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
> drivers/platform/x86/amd/pmf/tee-if.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 502ce93d5cdd..f8c0177afb0d 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -298,8 +298,10 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> if (!new_policy_buf)
> return -ENOMEM;
>
> - if (copy_from_user(new_policy_buf, buf, length))
> + if (copy_from_user(new_policy_buf, buf, length)) {
> + kfree(new_policy_buf);
> return -EFAULT;
> + }
>
> kfree(dev->policy_buf);
> dev->policy_buf = new_policy_buf;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-23 14:37 ` Shyam Sundar S K
@ 2024-01-24 1:29 ` Cong Liu
2024-01-26 19:16 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Cong Liu @ 2024-01-24 1:29 UTC (permalink / raw)
To: shyam-sundar.s-k, Shyam Sundar S K, Hans de Goede,
Ilpo Järvinen, Mario Limonciello
Cc: linux-kernel, liucong2, platform-driver-x86
amd_pmf_get_pb_data() will allocate memory for the policy buffer,
but does not free it if copy_from_user() fails. This leads to a memory
leak.
Fixes: 10817f28e533 ("platform/x86/amd/pmf: Add capability to sideload of policy binary")
Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
drivers/platform/x86/amd/pmf/tee-if.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 502ce93d5cdd..f8c0177afb0d 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -298,8 +298,10 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
if (!new_policy_buf)
return -ENOMEM;
- if (copy_from_user(new_policy_buf, buf, length))
+ if (copy_from_user(new_policy_buf, buf, length)) {
+ kfree(new_policy_buf);
return -EFAULT;
+ }
kfree(dev->policy_buf);
dev->policy_buf = new_policy_buf;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-24 1:29 ` [PATCH v2] " Cong Liu
@ 2024-01-26 19:16 ` Hans de Goede
2024-01-28 10:45 ` [v2] " Markus Elfring
0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2024-01-26 19:16 UTC (permalink / raw)
To: Cong Liu, shyam-sundar.s-k, Ilpo Järvinen, Mario Limonciello
Cc: linux-kernel, platform-driver-x86
Hi,
On 1/24/24 02:29, Cong Liu wrote:
> amd_pmf_get_pb_data() will allocate memory for the policy buffer,
> but does not free it if copy_from_user() fails. This leads to a memory
> leak.
>
> Fixes: 10817f28e533 ("platform/x86/amd/pmf: Add capability to sideload of policy binary")
> Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
Thank you for your patch/series, I've applied this patch
(series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in the pdx86 review-hans branch once I've
pushed my local branch there, which might take a while.
I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
> ---
> drivers/platform/x86/amd/pmf/tee-if.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 502ce93d5cdd..f8c0177afb0d 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -298,8 +298,10 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> if (!new_policy_buf)
> return -ENOMEM;
>
> - if (copy_from_user(new_policy_buf, buf, length))
> + if (copy_from_user(new_policy_buf, buf, length)) {
> + kfree(new_policy_buf);
> return -EFAULT;
> + }
>
> kfree(dev->policy_buf);
> dev->policy_buf = new_policy_buf;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-26 19:16 ` Hans de Goede
@ 2024-01-28 10:45 ` Markus Elfring
2024-01-29 8:59 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Markus Elfring @ 2024-01-28 10:45 UTC (permalink / raw)
To: Hans de Goede, Cong Liu, Shyam Sundar S K, Mario Limonciello,
Ilpo Järvinen, platform-driver-x86, kernel-janitors
Cc: LKML
> Thank you for your patch/series, I've applied this patch
> (series) to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> Note it will show up in the pdx86 review-hans branch once I've
> pushed my local branch there, which might take a while.
Will development interests grow for the application of known scripts
also according to the semantic patch language?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?h=v6.8-rc1#n71
Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> make COCCI=scripts/coccinelle/api/memdup_user.cocci M=drivers/platform/x86/amd/pmf/ coccicheck
…
drivers/platform/x86/amd/pmf/tee-if.c:297:18-25: WARNING opportunity for memdup_user
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-28 10:45 ` [v2] " Markus Elfring
@ 2024-01-29 8:59 ` Hans de Goede
2024-01-29 9:52 ` Markus Elfring
0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2024-01-29 8:59 UTC (permalink / raw)
To: Markus Elfring, Cong Liu, Shyam Sundar S K, Mario Limonciello,
Ilpo Järvinen, platform-driver-x86, kernel-janitors
Cc: LKML
Hi,
On 1/28/24 11:45, Markus Elfring wrote:
>> Thank you for your patch/series, I've applied this patch
>> (series) to my review-hans branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>>
>> Note it will show up in the pdx86 review-hans branch once I've
>> pushed my local branch there, which might take a while.
>
> Will development interests grow for the application of known scripts
> also according to the semantic patch language?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?h=v6.8-rc1#n71
Markus,
I'm not sure what your question here is?
Is it: "Will coccinelle scripts be run as part of the regular patch
test/merge workflow?" then the answer is that there are no plans
that I'm aware of to do that at this moment.
If such a thing were to be done, IMHO it would be best to have one
of the existing CI systems like e.h. Intel's LKP test bot run this
on linux-next, or on all the trees LKP already monitors.
And it does sound like something interesting to do, but someone
would need to actually setup and maintain such a CI system.
If the question is: "Are patches generated by coccinelle welcome?"
then the answer is "Yes patches generated by coccinelle are very
much welcome".
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data()
2024-01-29 8:59 ` Hans de Goede
@ 2024-01-29 9:52 ` Markus Elfring
0 siblings, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2024-01-29 9:52 UTC (permalink / raw)
To: Hans de Goede, Cong Liu, Shyam Sundar S K, Mario Limonciello,
Ilpo Järvinen, platform-driver-x86, kernel-janitors
Cc: LKML
> If the question is: "Are patches generated by coccinelle welcome?"
> then the answer is "Yes patches generated by coccinelle are very
> much welcome".
How do you think about to fix a questionable memory leak
by using the function “memdup_user” instead?
https://elixir.bootlin.com/linux/v6.8-rc1/source/mm/util.c#L185
Would you like to try a corresponding command out once more on source files
of a software like “Linux next-20240125”?
https://elixir.bootlin.com/linux/v6.8-rc1/source/scripts/coccinelle/api/memdup_user.cocci#L2
make COCCI=scripts/coccinelle/api/memdup_user.cocci M=drivers/platform/x86/amd/pmf/ MODE=patch coccicheck
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-29 9:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-23 1:11 [PATCH] platform/x86/amd/pmf: Fix memory leak in amd_pmf_get_pb_data() Cong Liu
2024-01-23 14:37 ` Shyam Sundar S K
2024-01-24 1:29 ` [PATCH v2] " Cong Liu
2024-01-26 19:16 ` Hans de Goede
2024-01-28 10:45 ` [v2] " Markus Elfring
2024-01-29 8:59 ` Hans de Goede
2024-01-29 9:52 ` Markus Elfring
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).