Patchwork [v1,1/5] crypto: ccp: Fix command completion detection race

login
register
mail settings
Submitter Tom Lendacky
Date July 3, 2018, 5:11 p.m.
Message ID <20180703171133.3225.66397.stgit@tlendack-t1.amdoffice.net>
Download mbox | patch
Permalink /patch/10504787/
State Accepted
Delegated to: Herbert Xu
Headers show

Comments

Tom Lendacky - July 3, 2018, 5:11 p.m.
The wait_event() function is used to detect command completion.  The
interrupt handler will set the wait condition variable when the interrupt
is triggered.  However, the variable used for wait_event() is initialized
after the command has been submitted, which can create a race condition
with the interrupt handler and result in the wait_event() never returning.
Move the initialization of the wait condition variable to just before
command submission.

Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
Cc: <stable@vger.kernel.org> # 4.16.x-
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/crypto/ccp/psp-dev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Brijesh Singh - July 5, 2018, 3:43 p.m.
On 07/03/2018 12:11 PM, Tom Lendacky wrote:
> The wait_event() function is used to detect command completion.  The
> interrupt handler will set the wait condition variable when the interrupt
> is triggered.  However, the variable used for wait_event() is initialized
> after the command has been submitted, which can create a race condition
> with the interrupt handler and result in the wait_event() never returning.
> Move the initialization of the wait condition variable to just before
> command submission.
> 
> Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
> Cc: <stable@vger.kernel.org> # 4.16.x-
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


>   drivers/crypto/ccp/psp-dev.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index ff478d8..973d683 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -84,8 +84,6 @@ static irqreturn_t psp_irq_handler(int irq, void *data)
>   
>   static void sev_wait_cmd_ioc(struct psp_device *psp, unsigned int *reg)
>   {
> -	psp->sev_int_rcvd = 0;
> -
>   	wait_event(psp->sev_int_queue, psp->sev_int_rcvd);
>   	*reg = ioread32(psp->io_regs + PSP_CMDRESP);
>   }
> @@ -148,6 +146,8 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
>   	iowrite32(phys_lsb, psp->io_regs + PSP_CMDBUFF_ADDR_LO);
>   	iowrite32(phys_msb, psp->io_regs + PSP_CMDBUFF_ADDR_HI);
>   
> +	psp->sev_int_rcvd = 0;
> +
>   	reg = cmd;
>   	reg <<= PSP_CMDRESP_CMD_SHIFT;
>   	reg |= PSP_CMDRESP_IOC;
>
Gary R Hook - July 5, 2018, 6:23 p.m.
On 07/03/2018 12:11 PM, Tom Lendacky wrote:
> The wait_event() function is used to detect command completion.  The
> interrupt handler will set the wait condition variable when the interrupt
> is triggered.  However, the variable used for wait_event() is initialized
> after the command has been submitted, which can create a race condition
> with the interrupt handler and result in the wait_event() never returning.
> Move the initialization of the wait condition variable to just before
> command submission.
> 
> Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
> Cc: <stable@vger.kernel.org> # 4.16.x-
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Acked-by: Gary R Hook <gary.hook@amd.com>

> ---
>   drivers/crypto/ccp/psp-dev.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index ff478d8..973d683 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -84,8 +84,6 @@ static irqreturn_t psp_irq_handler(int irq, void *data)
>   
>   static void sev_wait_cmd_ioc(struct psp_device *psp, unsigned int *reg)
>   {
> -	psp->sev_int_rcvd = 0;
> -
>   	wait_event(psp->sev_int_queue, psp->sev_int_rcvd);
>   	*reg = ioread32(psp->io_regs + PSP_CMDRESP);
>   }
> @@ -148,6 +146,8 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
>   	iowrite32(phys_lsb, psp->io_regs + PSP_CMDBUFF_ADDR_LO);
>   	iowrite32(phys_msb, psp->io_regs + PSP_CMDBUFF_ADDR_HI);
>   
> +	psp->sev_int_rcvd = 0;
> +
>   	reg = cmd;
>   	reg <<= PSP_CMDRESP_CMD_SHIFT;
>   	reg |= PSP_CMDRESP_IOC;
>
Gary R Hook - July 5, 2018, 6:24 p.m.
On 07/05/2018 10:43 AM, Brijesh Singh wrote:
> 
> 
> On 07/03/2018 12:11 PM, Tom Lendacky wrote:
>> The wait_event() function is used to detect command completion.  The
>> interrupt handler will set the wait condition variable when the interrupt
>> is triggered.  However, the variable used for wait_event() is initialized
>> after the command has been submitted, which can create a race condition
>> with the interrupt handler and result in the wait_event() never 
>> returning.
>> Move the initialization of the wait condition variable to just before
>> command submission.
>>
>> Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization 
>> (SEV) command support")
>> Cc: <stable@vger.kernel.org> # 4.16.x-
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
> 
> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

Acked-by: Gary R Hook <gary.hook@amd.com>

> 
> 
>>   drivers/crypto/ccp/psp-dev.c |    4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
>> index ff478d8..973d683 100644
>> --- a/drivers/crypto/ccp/psp-dev.c
>> +++ b/drivers/crypto/ccp/psp-dev.c
>> @@ -84,8 +84,6 @@ static irqreturn_t psp_irq_handler(int irq, void *data)
>>   static void sev_wait_cmd_ioc(struct psp_device *psp, unsigned int *reg)
>>   {
>> -    psp->sev_int_rcvd = 0;
>> -
>>       wait_event(psp->sev_int_queue, psp->sev_int_rcvd);
>>       *reg = ioread32(psp->io_regs + PSP_CMDRESP);
>>   }
>> @@ -148,6 +146,8 @@ static int __sev_do_cmd_locked(int cmd, void 
>> *data, int *psp_ret)
>>       iowrite32(phys_lsb, psp->io_regs + PSP_CMDBUFF_ADDR_LO);
>>       iowrite32(phys_msb, psp->io_regs + PSP_CMDBUFF_ADDR_HI);
>> +    psp->sev_int_rcvd = 0;
>> +
>>       reg = cmd;
>>       reg <<= PSP_CMDRESP_CMD_SHIFT;
>>       reg |= PSP_CMDRESP_IOC;
>>

Patch

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index ff478d8..973d683 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -84,8 +84,6 @@  static irqreturn_t psp_irq_handler(int irq, void *data)
 
 static void sev_wait_cmd_ioc(struct psp_device *psp, unsigned int *reg)
 {
-	psp->sev_int_rcvd = 0;
-
 	wait_event(psp->sev_int_queue, psp->sev_int_rcvd);
 	*reg = ioread32(psp->io_regs + PSP_CMDRESP);
 }
@@ -148,6 +146,8 @@  static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
 	iowrite32(phys_lsb, psp->io_regs + PSP_CMDBUFF_ADDR_LO);
 	iowrite32(phys_msb, psp->io_regs + PSP_CMDBUFF_ADDR_HI);
 
+	psp->sev_int_rcvd = 0;
+
 	reg = cmd;
 	reg <<= PSP_CMDRESP_CMD_SHIFT;
 	reg |= PSP_CMDRESP_IOC;