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

2021-04-04 Thread Christophe Leroy




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 
Cc: Borislav Petkov 
Cc: Tom Lendacky 
Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command 
support")
Signed-off-by: Sean Christopherson 
---
  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;



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

2021-04-02 Thread Sean Christopherson
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 
Cc: Borislav Petkov 
Cc: Tom Lendacky 
Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) 
command support")
Signed-off-by: Sean Christopherson 
---
 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;
+
/* 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;
-- 
2.31.0.208.g409f899ff0-goog