* [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
@ 2021-06-15 10:16 Kai Huang
2021-06-15 13:20 ` Jarkko Sakkinen
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kai Huang @ 2021-06-15 10:16 UTC (permalink / raw)
To: linux-sgx, x86
Cc: linux-kernel, bp, seanjc, jarkko, dave.hansen, tglx, mingo,
Yang Zhong, Kai Huang
xa_destroy() needs to be called to destroy virtual EPC's page arra
before calling kfree() to free the virtual EPC. Currently it is not
calaled. Add the missing xa_destroy() to fix.
Fixes: 540745ddbc70 ("x86/sgx: Introduce virtual EPC for use by KVM guests")
Tested-by: Yang Zhong <yang.zhong@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
arch/x86/kernel/cpu/sgx/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index 6ad165a5c0cc..64511c4a5200 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -212,6 +212,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
list_splice_tail(&secs_pages, &zombie_secs_pages);
mutex_unlock(&zombie_secs_pages_lock);
+ xa_destroy(&vepc->page_array);
kfree(vepc);
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-15 10:16 [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed Kai Huang
@ 2021-06-15 13:20 ` Jarkko Sakkinen
2021-06-16 0:30 ` Kai Huang
2021-06-15 15:39 ` Dave Hansen
2021-06-15 16:16 ` [tip: x86/urgent] " tip-bot2 for Kai Huang
2 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2021-06-15 13:20 UTC (permalink / raw)
To: Kai Huang
Cc: linux-sgx, x86, linux-kernel, bp, seanjc, dave.hansen, tglx,
mingo, Yang Zhong
On Tue, Jun 15, 2021 at 10:16:39PM +1200, Kai Huang wrote:
> xa_destroy() needs to be called to destroy virtual EPC's page arra
y
> before calling kfree() to free the virtual EPC. Currently it is not
> calaled. Add the missing xa_destroy() to fix.
called
> Fixes: 540745ddbc70 ("x86/sgx: Introduce virtual EPC for use by KVM guests")
> Tested-by: Yang Zhong <yang.zhong@intel.com>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
> arch/x86/kernel/cpu/sgx/virt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
> index 6ad165a5c0cc..64511c4a5200 100644
> --- a/arch/x86/kernel/cpu/sgx/virt.c
> +++ b/arch/x86/kernel/cpu/sgx/virt.c
> @@ -212,6 +212,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
> list_splice_tail(&secs_pages, &zombie_secs_pages);
> mutex_unlock(&zombie_secs_pages_lock);
>
> + xa_destroy(&vepc->page_array);
> kfree(vepc);
>
> return 0;
> --
> 2.31.1
>
>
/Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-15 10:16 [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed Kai Huang
2021-06-15 13:20 ` Jarkko Sakkinen
@ 2021-06-15 15:39 ` Dave Hansen
2021-06-16 0:28 ` Kai Huang
2021-06-15 16:16 ` [tip: x86/urgent] " tip-bot2 for Kai Huang
2 siblings, 1 reply; 9+ messages in thread
From: Dave Hansen @ 2021-06-15 15:39 UTC (permalink / raw)
To: Kai Huang, linux-sgx, x86
Cc: linux-kernel, bp, seanjc, jarkko, tglx, mingo, Yang Zhong
On 6/15/21 3:16 AM, Kai Huang wrote:
> xa_destroy() needs to be called to destroy virtual EPC's page arra
> before calling kfree() to free the virtual EPC. Currently it is not
> calaled. Add the missing xa_destroy() to fix.
Looks good Kai, thanks for fixing this.
Could you please take a good look through the sgx_release() and the vpec
equivalent and see if anything else stands out as possibly being missed?
Also, is this the kind of thing that a simple open/add/close selftest
might have found?
Maybe we should beef up the selftests a bit.
Acked-by: Dave Hansen <dave.hansen@intel.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip: x86/urgent] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-15 10:16 [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed Kai Huang
2021-06-15 13:20 ` Jarkko Sakkinen
2021-06-15 15:39 ` Dave Hansen
@ 2021-06-15 16:16 ` tip-bot2 for Kai Huang
2 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Kai Huang @ 2021-06-15 16:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: Kai Huang, Borislav Petkov, Dave Hansen, Yang Zhong, x86, linux-kernel
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 4692bc775d2180a937335ccba0edce557103d44a
Gitweb: https://git.kernel.org/tip/4692bc775d2180a937335ccba0edce557103d44a
Author: Kai Huang <kai.huang@intel.com>
AuthorDate: Tue, 15 Jun 2021 22:16:39 +12:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 15 Jun 2021 18:03:45 +02:00
x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
xa_destroy() needs to be called to destroy a virtual EPC's page array
before calling kfree() to free the virtual EPC. Currently it is not
called so add the missing xa_destroy().
Fixes: 540745ddbc70 ("x86/sgx: Introduce virtual EPC for use by KVM guests")
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Dave Hansen <dave.hansen@intel.com>
Tested-by: Yang Zhong <yang.zhong@intel.com>
Link: https://lkml.kernel.org/r/20210615101639.291929-1-kai.huang@intel.com
---
arch/x86/kernel/cpu/sgx/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index 6ad165a..64511c4 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -212,6 +212,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
list_splice_tail(&secs_pages, &zombie_secs_pages);
mutex_unlock(&zombie_secs_pages_lock);
+ xa_destroy(&vepc->page_array);
kfree(vepc);
return 0;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-15 15:39 ` Dave Hansen
@ 2021-06-16 0:28 ` Kai Huang
0 siblings, 0 replies; 9+ messages in thread
From: Kai Huang @ 2021-06-16 0:28 UTC (permalink / raw)
To: Dave Hansen, linux-sgx, x86
Cc: linux-kernel, bp, seanjc, jarkko, tglx, mingo, Yang Zhong
On Tue, 2021-06-15 at 08:39 -0700, Dave Hansen wrote:
> On 6/15/21 3:16 AM, Kai Huang wrote:
> > xa_destroy() needs to be called to destroy virtual EPC's page arra
> > before calling kfree() to free the virtual EPC. Currently it is not
> > calaled. Add the missing xa_destroy() to fix.
>
> Looks good Kai, thanks for fixing this.
>
> Could you please take a good look through the sgx_release() and the vpec
> equivalent and see if anything else stands out as possibly being missed?
I looked over. One potential issue is both sgx_encl and sgx_vepc have 'struct mutex lock'
embedded, but mutex_destroy() is not called when they are released. However I am not
sure whether this is worth fixing, since mutex_destroy() is empty unless
CONFIG_DEBUG_MUTEXES is turned on (even with it turned on, mutex_destroy() doesn't do
anything like freeing resources so in practice there should have no problem).
Another thing is sgx_encl_release() doesn't explicitly call xa_erase() for each EPC page
in encl->page_array when looping it over to free all EPC pages, but I think it is OK since
xa_destroy() is called later which will destroy all xarray internal data structures. But
I don't know internal implementation of xarray.
> Also, is this the kind of thing that a simple open/add/close selftest
> might have found?
It might be useful but I don't think it can detect things like xa_destroy() being missing.
>
> Maybe we should beef up the selftests a bit.
>
> Acked-by: Dave Hansen <dave.hansen@intel.com>
Thank you!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-15 13:20 ` Jarkko Sakkinen
@ 2021-06-16 0:30 ` Kai Huang
2021-06-17 14:34 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: Kai Huang @ 2021-06-16 0:30 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-sgx, x86, linux-kernel, bp, seanjc, dave.hansen, tglx,
mingo, Yang Zhong
On Tue, 2021-06-15 at 16:20 +0300, Jarkko Sakkinen wrote:
> On Tue, Jun 15, 2021 at 10:16:39PM +1200, Kai Huang wrote:
> > xa_destroy() needs to be called to destroy virtual EPC's page arra
> y
>
> > before calling kfree() to free the virtual EPC. Currently it is not
> > calaled. Add the missing xa_destroy() to fix.
> called
Thanks Jarkko. I literally need to find some way to avoid such error in future :)
>
> > Fixes: 540745ddbc70 ("x86/sgx: Introduce virtual EPC for use by KVM guests")
> > Tested-by: Yang Zhong <yang.zhong@intel.com>
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > ---
> > arch/x86/kernel/cpu/sgx/virt.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
> > index 6ad165a5c0cc..64511c4a5200 100644
> > --- a/arch/x86/kernel/cpu/sgx/virt.c
> > +++ b/arch/x86/kernel/cpu/sgx/virt.c
> > @@ -212,6 +212,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
> > list_splice_tail(&secs_pages, &zombie_secs_pages);
> > mutex_unlock(&zombie_secs_pages_lock);
> >
> > + xa_destroy(&vepc->page_array);
> > kfree(vepc);
> >
> > return 0;
> > --
> > 2.31.1
> >
> >
>
> /Jarkko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-16 0:30 ` Kai Huang
@ 2021-06-17 14:34 ` Borislav Petkov
2021-06-18 0:04 ` Kai Huang
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2021-06-17 14:34 UTC (permalink / raw)
To: Kai Huang
Cc: Jarkko Sakkinen, linux-sgx, x86, linux-kernel, seanjc,
dave.hansen, tglx, mingo, Yang Zhong
On Wed, Jun 16, 2021 at 12:30:04PM +1200, Kai Huang wrote:
> Thanks Jarkko. I literally need to find some way to avoid such error in future :)
That way is called "integrate checkpatch.pl into your patch creation
workflow".
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-17 14:34 ` Borislav Petkov
@ 2021-06-18 0:04 ` Kai Huang
2021-06-18 5:15 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: Kai Huang @ 2021-06-18 0:04 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jarkko Sakkinen, linux-sgx, x86, linux-kernel, seanjc,
dave.hansen, tglx, mingo, Yang Zhong
On Thu, 2021-06-17 at 16:34 +0200, Borislav Petkov wrote:
> On Wed, Jun 16, 2021 at 12:30:04PM +1200, Kai Huang wrote:
> > Thanks Jarkko. I literally need to find some way to avoid such error in future :)
>
> That way is called "integrate checkpatch.pl into your patch creation
> workflow".
>
Thanks for suggestion. Yes I actually did the checkpatch.pl, but it didn't report typo in
commit message. A little bit strange.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed
2021-06-18 0:04 ` Kai Huang
@ 2021-06-18 5:15 ` Borislav Petkov
0 siblings, 0 replies; 9+ messages in thread
From: Borislav Petkov @ 2021-06-18 5:15 UTC (permalink / raw)
To: Kai Huang
Cc: Jarkko Sakkinen, linux-sgx, x86, linux-kernel, seanjc,
dave.hansen, tglx, mingo, Yang Zhong
On Fri, Jun 18, 2021 at 12:04:55PM +1200, Kai Huang wrote:
> Thanks for suggestion. Yes I actually did the checkpatch.pl, but it
> didn't report typo in commit message. A little bit strange.
Yah, and I know it does catch typos. It seems it does so only sometimes:
$ ./scripts/checkpatch.pl --strict /tmp/kai.01
total: 0 errors, 0 warnings, 0 checks, 7 lines checked
/tmp/kai.01 has no obvious style problems and is ready for submission.
Someday soon we'll have a better way to deal with this.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-06-18 5:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 10:16 [PATCH] x86/sgx: Add missing xa_destroy() when virtual EPC is destroyed Kai Huang
2021-06-15 13:20 ` Jarkko Sakkinen
2021-06-16 0:30 ` Kai Huang
2021-06-17 14:34 ` Borislav Petkov
2021-06-18 0:04 ` Kai Huang
2021-06-18 5:15 ` Borislav Petkov
2021-06-15 15:39 ` Dave Hansen
2021-06-16 0:28 ` Kai Huang
2021-06-15 16:16 ` [tip: x86/urgent] " tip-bot2 for Kai Huang
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).