From: Li Fei1 <fei1.li@intel.com>
To: zhou1615@umn.edu, kjlu@umn.edu, reinette.chatre@intel.com,
zhi.a.wang@intel.com, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, fei1.li@intel.com
Subject: Re: [PATCH] virt: acrn: fix a memory leak bug in acrn_dev_ioctl()
Date: Tue, 4 Jan 2022 16:09:02 +0800 [thread overview]
Message-ID: <20220104080902.GA7254@louislifei-OptiPlex-7050> (raw)
In-Reply-To: <YdP5N3lYqF09dLPv@kroah.com>
On Tue, Jan 04, 2022 at 08:37:27AM +0100, Greg KH wrote:
> On Tue, Jan 04, 2022 at 11:50:33AM +0800, Li Fei1 wrote:
> > On Tue, Jan 04, 2022 at 11:45:01AM +0800, Zhou Qingyang wrote:
> > > In acrn_dev_ioctl(), vm_param is not released or passed out on the
> > > error path of "if ((vm_param->reserved0 | vm_param->reserved1) != 0)",
> > > which could lead to a memory leak.
> > >
> > > Fix this bug by adding a kfree of vm_param on the error path.
> > >
> > > This bug was found by a static analyzer.
> > >
> > > Builds with CONFIG_ACRN_GUEST=y, CONFIG_ACRN_HSM=y show no new warnings,
> > > and our static analyzer no longer warns about this code.
> > >
> > > Fixes: 9c5137aedd11 (“9c5137aedd11 virt: acrn: Introduce VM management interfaces”)
> > > Signed-off-by: Zhou Qingyang <zhou1615@umn.edu>
> > > ---
> > > The analysis employs differential checking to identify inconsistent
> > > security operations (e.g., checks or kfrees) between two code paths
> > > and confirms that the inconsistent operations are not recovered in
> > > the current function or the callers, so they constitute bugs.
> > >
> > > Note that, as a bug found by static analysis, it can be a false
> > > positive or hard to trigger. Multiple researchers have cross-reviewed
> > > the bug.
> > >
> > > Changes in v2:
> > > - Fix the same bug in ACRN_IOCTL_SET_VCPU_REGS.
Hi Qingyang
I think the "Changes in v2" was not fixes the code in commit "9c5137aedd11 virt: acrn: Introduce VM management interfaces", but in commit 2ad2aaee1bc9 "virt: acrn: Introduce an ioctl to set vCPU registers state".
Would you please to split this patch into two and please replace “...” with "..." in your commit message.
Thanks.
> > >
> >
> > Signed-off-by: Fei Li <fei1.li@intel.com>
Please ignore my "Signed-off" here. Sorry.
>
> For the multiple obvious reasons (not the least being you didn't even
> run it through our normal testing scripts), I am not going to take this
> change, sorry.
>
> Fei, please be more careful in the future.
>
> greg k-h
prev parent reply other threads:[~2022-01-04 8:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-04 2:34 [PATCH] virt: acrn: fix a memory leak bug in acrn_dev_ioctl() Zhou Qingyang
2022-01-04 2:47 ` Li Fei1
2022-01-04 3:45 ` Zhou Qingyang
2022-01-04 3:50 ` Li Fei1
2022-01-04 7:37 ` Greg KH
2022-01-04 8:01 ` Li Fei1
2022-01-04 8:09 ` Li Fei1 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220104080902.GA7254@louislifei-OptiPlex-7050 \
--to=fei1.li@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=kjlu@umn.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=reinette.chatre@intel.com \
--cc=zhi.a.wang@intel.com \
--cc=zhou1615@umn.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.