2022-03-08 23:48:56

by butt3rflyh4ck

Subject: [PATCH] virt: acrn: fix a memory leak in acrn_dev_ioctl()

The vm_param and cpu_regs need to be freed via kfree()
before return -EINVAL error.

Fixes: 9c5137aedd11 ("virt: acrn: Introduce VM management interfaces")
Fixes: 2ad2aaee1bc9 ("virt: acrn: Introduce an ioctl to set vCPU registers state")
Signed-off-by: Xiaolong Huang <[email protected]>
Signed-off-by: Fei Li <[email protected]>
---
drivers/virt/acrn/hsm.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
index 5419794fccf1..423ea888d79a 100644
--- a/drivers/virt/acrn/hsm.c
+++ b/drivers/virt/acrn/hsm.c
@@ -136,8 +136,10 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
if (IS_ERR(vm_param))
return PTR_ERR(vm_param);

- if ((vm_param->reserved0 | vm_param->reserved1) != 0)
+ if ((vm_param->reserved0 | vm_param->reserved1) != 0) {
+ kfree(vm_param);
return -EINVAL;
+ }

vm = acrn_vm_create(vm, vm_param);
if (!vm) {
@@ -182,21 +184,29 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
return PTR_ERR(cpu_regs);

for (i = 0; i < ARRAY_SIZE(cpu_regs->reserved); i++)
- if (cpu_regs->reserved[i])
+ if (cpu_regs->reserved[i]) {
+ kfree(cpu_regs);
return -EINVAL;
+ }

for (i = 0; i < ARRAY_SIZE(cpu_regs->vcpu_regs.reserved_32); i++)
- if (cpu_regs->vcpu_regs.reserved_32[i])
+ if (cpu_regs->vcpu_regs.reserved_32[i]) {
+ kfree(cpu_regs);
return -EINVAL;
+ }

for (i = 0; i < ARRAY_SIZE(cpu_regs->vcpu_regs.reserved_64); i++)
- if (cpu_regs->vcpu_regs.reserved_64[i])
+ if (cpu_regs->vcpu_regs.reserved_64[i]) {
+ kfree(cpu_regs);
return -EINVAL;
+ }

for (i = 0; i < ARRAY_SIZE(cpu_regs->vcpu_regs.gdt.reserved); i++)
if (cpu_regs->vcpu_regs.gdt.reserved[i] |
- cpu_regs->vcpu_regs.idt.reserved[i])
+ cpu_regs->vcpu_regs.idt.reserved[i]) {
+ kfree(cpu_regs);
return -EINVAL;
+ }

ret = hcall_set_vcpu_regs(vm->vmid, virt_to_phys(cpu_regs));
if (ret < 0)

base-commit: 5859a2b1991101d6b978f3feb5325dad39421f29
--
2.25.1

2022-03-15 09:08:37

by Greg KH

Subject: Re: [PATCH] virt: acrn: fix a memory leak in acrn_dev_ioctl()

On Tue, Mar 15, 2022 at 03:36:11PM +0800, butt3rflyh4ck wrote:
> Ok, thanks. By the way, I want to explain, Firstly there is just some
> parameter that should be freed before func returns the -EINVAL error
> in the patch.

What patch? I see nothing here :(

> I think it was correct, no need to test it. And secondly the commitor
> Li Fei1 also reviewed the patch code. finally I am sorry that no arcn
> hso hardware to test it.

I still have no context at all. Please properly quote your email
threads so that we can all understand and respond properly.

thanks,

greg k-h

2022-03-15 15:52:29

by Greg KH

Subject: Re: [PATCH] virt: acrn: fix a memory leak in acrn_dev_ioctl()

On Tue, Mar 15, 2022 at 10:44:09AM +0800, butt3rflyh4ck wrote:
> Hi, Greg, could you tell me how to test (like this)?

I have no context at all what "like this" is sorry.

greg k-h

2022-03-16 19:08:16

by butt3rflyh4ck

Subject: Re: [PATCH] virt: acrn: fix a memory leak in acrn_dev_ioctl()

Ok, thanks. By the way, I want to explain, Firstly there is just some
parameter that should be freed before func returns the -EINVAL error
in the patch.
I think it was correct, no need to test it. And secondly the commitor
Li Fei1 also reviewed the patch code. finally I am sorry that no arcn
hso hardware to test it.

Regards,
butt3rflyh4ck.

On Tue, Mar 15, 2022 at 2:49 PM Greg KH <[email protected]> wrote:
>
> On Tue, Mar 15, 2022 at 10:44:09AM +0800, butt3rflyh4ck wrote:
> > Hi, Greg, could you tell me how to test (like this)?
>
> I have no context at all what "like this" is sorry.
>
> greg k-h



--
Active Defense Lab of Venustech