On 11/10/21 12:43 PM, Borislav Petkov wrote:

On Wed, Nov 10, 2021 at 08:21:21AM -0600, Brijesh Singh wrote:

quoted

I am assuming you mean add some compile time check to ensure that desc will
fit in the shared buffer ?

No:

struct ghcb {

	...

         u8 shared_buffer[2032];

so that memcpy needs to do:

	memcpy(ghcb->shared_buffer, desc, min_t(int, 2032, sizeof(*desc)));

with that 2032 behind a proper define, ofc.

2032 => sizeof(ghcb->shared_buffer) ?

The idea is that a full snp_psc_desc structure is meant to fit completely 
in the shared_buffer area. So if there are no compile time checks, then 
the code on the HV side will need to ensure that the input doesn't cause 
the HV to access the structure outside of the shared_buffer area - which, 
IIRC, it does (think protect against a malicious guest), so the min_t() on 
the memcpy should be safe on the guest side.

But given the snp_psc_desc is sized/meant to fit completely in the 
shared_buffer, a compile time check would be a good idea, too, right?

Thanks,
Tom

quoted

I can drop the overlap comment to avoid the confusion, as you pointed it
more of the future thing. Basically overlap is the below condition

set_memory_private(gfn=0, page_size=2m)
set_memory_private(gfn=10, page_size=4k)

The RMPUPDATE instruction will detect overlap on the second call and return
an error to the guest. After we add the support to track the page validation
state (either in bitmap or page flag), the second call will not be issued
and thus avoid an overlap errors. For now, we use the page_size=4k for all
the page state changes from the kernel.

Yah, sounds like the comment is not needed now. You could put this in
the commit message, though.

Thx.