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

Kai Huang
2022-05-11T22:00:14Z

> > > > >
> > > > > 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