Re: [RFC PATCH 1/4] x86/sgx: Do not free backing memory on ENCLS[ELDU] failure

[Kai Huang <kai.huang@intel.com>]

> > > > > 
> > > > > This part puzzles me in the pseudo-code.
> > > > > 
> > > > > The version is read first:
> > > > > 
> > > > > TMP_VER := DS:RDX[63:0];
> > > > > 
> > > > > Then there's MAC calculation, comparison,  and finally this check:
> > > > > 
> > > > > (* Check version before committing *)
> > > > > IF (DS:RDX ≠ 0)
> > > > >         THEN #GP(0);
> > > > > ELSE
> > > > >         DS:RDX := TMP_VER;
> > > > > FI;
> > > > > 
> > > > > For me it is a mystery what does zero the slot and in what condition
> > > > > it would be non-zero. Perhaps the #GP refers anyway to this check?
> > > > 
> 
> 
> We discussed this internally, and agree this part of pseudo code needs be  
> corrected/clarified.
> 
> Here is what we think was going on when ELDU invoked with PCMD of all  
> zeros: ELDU would check if the PCMD.SECINFO.FLAGS.PT is 0 which indicates  
> that the page being loaded is a PT_SECS, and the PAGEINFO.SECS is not  
> zero, then the instruction will #GP(0).  Thus, ELDU is behaving correctly  
> – it is an omission in the SDM pseudocode.
> 
> The version checking code above also need be clarified because the VA slot  
> would be cleared at this point and TMP_VER should be zero.

"VA slot would be cleared at this point" isn't accurate.  The VA slot itself is
still occupied at this point.  The original TMP_VER before the decryption is the
VA slot value stored by EWB, and after decryption it becomes 0, if the
decryption is correct.  The correct pseudo-code should be:

	IF (TMP_VER != 0)
		THEN #GP(0);
	ELSE
		DS:RDX	= TMP_VER;
	FI;

The check of TMP_VER against 0 is just an additional safe guard to make sure
decryption didn't fail.


-- 
Thanks,
-Kai