| Message ID | 20210402233702.3291792-2-seanjc@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | ccp: KVM: SVM: Use stack for SEV command buffers | expand |
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@amd.com> > Cc: Borislav Petkov <bp@suse.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support") > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > 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; >
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;
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@amd.com> Cc: Borislav Petkov <bp@suse.de> Cc: Tom Lendacky <thomas.lendacky@amd.com> Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support") Signed-off-by: Sean Christopherson <seanjc@google.com> --- drivers/crypto/ccp/sev-dev.c | 3 +++ 1 file changed, 3 insertions(+)