Re: [RFC PATCH v6 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 26, 2021, Dave Hansen wrote:
> On 2/26/21 4:14 AM, Kai Huang wrote:
> > @@ -404,7 +421,8 @@ void sgx_encl_release(struct kref *ref)
> >  			if (sgx_unmark_page_reclaimable(entry->epc_page))
> >  				continue;
> >  
> > -			sgx_free_epc_page(entry->epc_page);
> > +			if (!sgx_reset_epc_page(entry->epc_page))
> > +				sgx_free_epc_page(entry->epc_page);
> 
> Won't this leak the page?

Yep.

> I think that's fine; the page *IS* unusable if this happens.  But, the
> error message that will show up isn't super informative.  If this
> happened to a bunch of EPC pages, we'd be out of EPC with nothing to
> show for it.
> 
> We must give a more informative message saying that the page is leaked.
>  Ideally, we'd also make this debuggable by dumping out how many of
> these pages there have been somewhere.  That can wait, though, until we
> have some kind of stats coming out of the code (there's nothing now).  A
> comment to remind us to do this would be nice.

Eh, having debugged these several times, the WARN_ONCE in sgx_reset_epc_page()
is probably sufficient.  IIRC, when I hit this, things were either laughably
broken and every page was failing, or there was another ENCLS failure somewhere
else that provided additional info.  Not saying don't add more debug info,
rather that it's probably not a priority.

> Anyway, these are in decent shape and only getting better.  It's time to
> get some more eyeballs on them and get the RFC tag off, so assuming that
> a better error message gets stuck in here:
> 
> Acked-by: Dave Hansen <dave.hansen@xxxxxxxxx>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux