crypto: ccp - Fix leaking the same page twice

[PATCH] crypto: ccp - Fix leaking the same page twice

Posted by Guenter Roeck 1 month ago

Commit 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is
missed") fixed a case where SNP is left in INIT state if page reclaim
fails. It removes the transition to the INIT state for this command and
adjusts the page state management.

While doing this, it added a call to snp_leak_pages() after a call to
snp_reclaim_pages() failed. Since snp_reclaim_pages() already calls
snp_leak_pages() internally on the pages it fails to reclaim, calling
it again leaks the exact same page twice.

Fix by removing the extra call to snp_leak_pages().

The problem was found by an experimental code review agent based on
gemini-3.1-pro while reviewing backports into v6.18.y.

Assisted-by: Gemini:gemini-3.1-pro
Fixes: 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is missed")
Cc: Tycho Andersen (AMD) <tycho@kernel.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/crypto/ccp/sev-dev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 096f993974d1..bd31ebfc85d5 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -2410,10 +2410,8 @@ static int sev_ioctl_do_snp_platform_status(struct sev_issue_cmd *argp)
 		 * in Firmware state on failure. Use snp_reclaim_pages() to
 		 * transition either case back to Hypervisor-owned state.
 		 */
-		if (snp_reclaim_pages(__pa(data), 1, true)) {
-			snp_leak_pages(__page_to_pfn(status_page), 1);
+		if (snp_reclaim_pages(__pa(data), 1, true))
 			return -EFAULT;
-		}
 	}
 
 	if (ret)
-- 
2.45.2

Re: [PATCH] crypto: ccp - Fix leaking the same page twice

Posted by Herbert Xu 3 weeks, 5 days ago

On Wed, Mar 04, 2026 at 12:39:34PM -0800, Guenter Roeck wrote:
> Commit 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is
> missed") fixed a case where SNP is left in INIT state if page reclaim
> fails. It removes the transition to the INIT state for this command and
> adjusts the page state management.
> 
> While doing this, it added a call to snp_leak_pages() after a call to
> snp_reclaim_pages() failed. Since snp_reclaim_pages() already calls
> snp_leak_pages() internally on the pages it fails to reclaim, calling
> it again leaks the exact same page twice.
> 
> Fix by removing the extra call to snp_leak_pages().
> 
> The problem was found by an experimental code review agent based on
> gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> Assisted-by: Gemini:gemini-3.1-pro
> Fixes: 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is missed")
> Cc: Tycho Andersen (AMD) <tycho@kernel.org>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/crypto/ccp/sev-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: ccp - Fix leaking the same page twice

Posted by Tycho Andersen 1 month ago

On Wed, Mar 04, 2026 at 12:39:34PM -0800, Guenter Roeck wrote:
> Commit 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is
> missed") fixed a case where SNP is left in INIT state if page reclaim
> fails. It removes the transition to the INIT state for this command and
> adjusts the page state management.
> 
> While doing this, it added a call to snp_leak_pages() after a call to
> snp_reclaim_pages() failed. Since snp_reclaim_pages() already calls
> snp_leak_pages() internally on the pages it fails to reclaim, calling
> it again leaks the exact same page twice.
> 
> Fix by removing the extra call to snp_leak_pages().

Reviewed-by: Tycho Andersen (AMD) <tycho@kernel.org>

Re: [PATCH] crypto: ccp - Fix leaking the same page twice

Posted by Tom Lendacky 1 month ago

On 3/4/26 14:39, Guenter Roeck wrote:
> Commit 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is
> missed") fixed a case where SNP is left in INIT state if page reclaim
> fails. It removes the transition to the INIT state for this command and
> adjusts the page state management.
> 
> While doing this, it added a call to snp_leak_pages() after a call to
> snp_reclaim_pages() failed. Since snp_reclaim_pages() already calls
> snp_leak_pages() internally on the pages it fails to reclaim, calling
> it again leaks the exact same page twice.
> 
> Fix by removing the extra call to snp_leak_pages().
> 
> The problem was found by an experimental code review agent based on
> gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> Assisted-by: Gemini:gemini-3.1-pro
> Fixes: 551120148b67 ("crypto: ccp - Fix a case where SNP_SHUTDOWN is missed")
> Cc: Tycho Andersen (AMD) <tycho@kernel.org>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 096f993974d1..bd31ebfc85d5 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2410,10 +2410,8 @@ static int sev_ioctl_do_snp_platform_status(struct sev_issue_cmd *argp)
>  		 * in Firmware state on failure. Use snp_reclaim_pages() to
>  		 * transition either case back to Hypervisor-owned state.
>  		 */
> -		if (snp_reclaim_pages(__pa(data), 1, true)) {
> -			snp_leak_pages(__page_to_pfn(status_page), 1);
> +		if (snp_reclaim_pages(__pa(data), 1, true))
>  			return -EFAULT;
> -		}
>  	}
>  
>  	if (ret)