Re: [PATCH 1/5] crypto: ccp: Detect and reject vmalloc addresses destined for PSP

From: Christophe Leroy
Date: Sun Apr 04 2021 - 02:32:11 EST




Le 03/04/2021 à 01:36, Sean Christopherson a écrit :
Explicitly reject vmalloc'd data as the source for SEV commands that are
sent to the PSP. The PSP works with physical addresses, and __pa() will
not return the correct address for a vmalloc'd pionter, which at best
will cause the command to fail, and at worst lead to system instability.

While it's unlikely that callers will deliberately use vmalloc() for SEV
buffers, a caller can easily use a vmalloc'd pointer unknowingly when
running with CONFIG_VMAP_STACK=y as it's not obvious that putting the
command buffers on the stack would be bad. The command buffers are
relative small and easily fit on the stack, and the APIs to do not
document that the incoming pointer must be a physically contiguous,
__pa() friendly pointer.

Cc: Brijesh Singh <brijesh.singh@xxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxx>
Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
drivers/crypto/ccp/sev-dev.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index cb9b4c4e371e..6556d220713b 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -150,6 +150,9 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
sev = psp->sev_data;
+ if (data && WARN_ON_ONCE(is_vmalloc_addr(data)))
+ return -EINVAL;
+

I hadn't seen this patch when I commented the 2 other ones, I received it only this night.

As commented in the other patches, is_vmalloc_addr() is not the best way to test that __pa() can be safely used.

For that, you have virt_addr_valid()

/* Get the physical address of the command buffer */
phys_lsb = data ? lower_32_bits(__psp_pa(data)) : 0;
phys_msb = data ? upper_32_bits(__psp_pa(data)) : 0;