kernel/vtpm: fix potential UB when accessing guest buffer

[stefano-garzarella]

Creating a mutable reference to the guest buffer, could have been an UB because the guest could potentially modify it.

So, let's revoke buffer access to the guest before creating the mutable reference with from_raw_parts_mut().

Restore buffer access to the guest, after the reference to the buffer is dropped (internal block in the match).

Suggested-by: Claudio Carvalho [email protected]
Suggested-by: Carlos López [email protected]

[stefano-garzarella]

This is a followup of #574 with a reference to the discussion we had here: #574 (comment) (thanks @00xc and @cclaudio)

I'm reporting the last comment from Carlos, to restart the discussion:

00xc Jan 16, 2025 •

I'd say so, yes (although someone can correct me on the need for a TLB flush on both operations). We would also need to revert the RMP change on the error path, and that in turn might require holding PVALIDATE_LOCK (although I'm foggy on the attack vector that lock prevented, @joergroedel remembers perhaps). Anyhow, this can be handled in the review for the separate PR.

I handled the error path, moving the error handling after the section, not sure what to do with PVALIDATE_LOCK.
@joergroedel any suggestion here?

[msft-jlange]

I don't think this is the sort of pattern we want to develop for interaction with guest memory. This change will revoke all guest access to the page that is being used by the TPM, and depending on how the guest is built (where it may be reading data, polling for memory changes, etc.), the guest may encounter an unexpected and fatal memory exception as a result of the permission change.

This pattern of modifying guest memory from the SVSM will be increasingly common as the SVSM begins to implement paravisor functionality. A much better pattern will be for the SVSM to be written defensively, using atomics or other Sync-safe accesses that assume the possibility for concurrent access - this is preferable to just assuming mutable access to the data and hoping for the best. If that's difficult, then consider copying the guest memory into a local SVSM buffer that can be accessed mutably for the operation, where the contents can be copied back to guest memory after the operation completes.