[PATCH v1 0/4] Fix some bugs in the CCP driver

[PATCH v1 0/4] Fix some bugs in the CCP driver

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

This set of fixes came out of using various AI tools on the SNP shutdown
series:
https://lore.kernel.org/all/20260326161110.1764303-1-tycho@kernel.org/

I'm not quite sure how to cite these tools yet, checkpatch complained a
bit about my citations here. Happy to respin if there's a better way. It
looks like e.g. Assisted-by in the process of being added:
https://lore.kernel.org/all/20260302143659.41882-1-thomas.hellstrom@linux.intel.com/

Thanks,

Tycho

Tycho Andersen (AMD) (4):
  crypto/ccp: Reverse the cleanup order in psp_dev_destroy()
  crypto/ccp: Fix snp_filter_reserved_mem_regions() off-by-one
  crypto/ccp: Check for page allocation failure correctly in TIO
  crypto/ccp: Initialize data during __sev_snp_init_locked()

 drivers/crypto/ccp/psp-dev.c |  8 ++++----
 drivers/crypto/ccp/sev-dev.c | 23 +++++++++++++----------
 2 files changed, 17 insertions(+), 14 deletions(-)


base-commit: 6c927e5ca9d238f8ae40b453a8382eb9cf4ee855
-- 
2.53.0

[PATCH v1 1/4] crypto/ccp: Reverse the cleanup order in psp_dev_destroy()

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

Before SNP x86 shutdown [1], all HV_FIXED pages were always leaked on
module unload. Now pages can be reclaimed if they are freed before SNP
shutdown.

The SFS driver does sfs_dev_destroy() -> snp_free_hv_fixed_pages(), marking
the command buffer as free. But this happens after sev_dev_destroy() in
psp_dev_destroy(), so the pages are always leaked.

Rearrange psp_dev_destroy() to destroy things in the reverse order from
psp_init(), so that any dependencies can be unwound accordingly. This lets
SFS free the page and the subsequent SNP shutdown release it.

This was identified with use of Chris Mason's review-prompts:
https://github.com/masoncl/review-prompts

[1]: https://lore.kernel.org/all/20260324161301.1353976-1-tycho@kernel.org/

Fixes: 648dbccc03a0 ("crypto: ccp - Add AMD Seamless Firmware Servicing (SFS) driver")
Reported-by: review-prompts
Assisted-by: Claude:claude-4.6-opus
Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/psp-dev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 5c7f7e02a7d8..b14ce51065d5 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -316,15 +316,15 @@ void psp_dev_destroy(struct sp_device *sp)
 	if (!psp)
 		return;
 
-	sev_dev_destroy(psp);
+	dbc_dev_destroy(psp);
 
-	tee_dev_destroy(psp);
+	platform_access_dev_destroy(psp);
 
 	sfs_dev_destroy(psp);
 
-	dbc_dev_destroy(psp);
+	tee_dev_destroy(psp);
 
-	platform_access_dev_destroy(psp);
+	sev_dev_destroy(psp);
 
 	sp_free_psp_irq(sp, psp);
 
-- 
2.53.0

[PATCH v1 2/4] crypto/ccp: Fix snp_filter_reserved_mem_regions() off-by-one

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

Sashiko notes:

> regarding the bounds check in snp_filter_reserved_mem_regions()
> called via walk_iomem_res_desc(): does the check
> if ((range_list->num_elements * 16 + 8) > PAGE_SIZE)
> allow an off-by-one heap buffer overflow?
>
> If range_list->num_elements is 255, 255 * 16 + 8 = 4088, which is <= 4096.
> Writing range->base (8 bytes) fills 4088-4095, but writing range->page_count
> (4 bytes) would write to 4096-4099, overflowing the kzalloc-allocated
> PAGE_SIZE buffer.

Fix this by accounting for the entry about to be written to, in addition to
the entries that are already allocated.

Fixes: 1ca5614b84ee ("crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 drivers/crypto/ccp/sev-dev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 939fa8aa155c..e87efcff8df2 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1328,10 +1328,11 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
 	size_t size;
 
 	/*
-	 * Ensure the list of HV_FIXED pages that will be passed to firmware
-	 * do not exceed the page-sized argument buffer.
+	 * Ensure the list of HV_FIXED pages passed to the firmware including
+	 * the one about to be written to do not exceed the page-sized argument
+	 * buffer.
 	 */
-	if ((range_list->num_elements * sizeof(struct sev_data_range) +
+	if (((range_list->num_elements + 1) * sizeof(struct sev_data_range) +
 	     sizeof(struct sev_data_range_list)) > PAGE_SIZE)
 		return -E2BIG;
 
-- 
2.53.0

[PATCH v1 3/4] crypto/ccp: Check for page allocation failure correctly in TIO

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

Sashiko notes:

> if __snp_alloc_firmware_pages() returns NULL under memory pressure, is it
> safe to pass it directly to page_address()?
>
> On architectures without HASHED_PAGE_VIRTUAL, page_address(NULL) might
> compute a deterministic but invalid, non-zero virtual address. The
> subsequent if (tio_status) check would then evaluate to true, and
> sev_tsm_init_locked() would dereference the invalid pointer.

Indeed, page_address(NULL) will return non-NULL garbage here. Fix this by
checking the page allocation itself for NULL, not the resulting virtual
address.

Fixes: 4be423572da1 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 drivers/crypto/ccp/sev-dev.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e87efcff8df2..11e2c667c0ad 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1488,6 +1488,8 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 				       &snp_panic_notifier);
 
 	if (data.tio_en) {
+		struct page *page;
+
 		/*
 		 * This executes with the sev_cmd_mutex held so down the stack
 		 * snp_reclaim_pages(locked=false) might be needed (which is extremely
@@ -1495,12 +1497,14 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 		 * Instead of exporting __snp_alloc_firmware_pages(), allocate a page
 		 * for this one call here.
 		 */
-		void *tio_status = page_address(__snp_alloc_firmware_pages(
-			GFP_KERNEL_ACCOUNT | __GFP_ZERO, 0, true));
+		page = __snp_alloc_firmware_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
+						  0, true);
+		if (page) {
+			void *tio_status = page_address(page);
 
-		if (tio_status) {
 			sev_tsm_init_locked(sev, tio_status);
-			__snp_free_firmware_pages(virt_to_page(tio_status), 0, true);
+
+			__snp_free_firmware_pages(page, 0, true);
 		}
 	}
 
-- 
2.53.0

[PATCH v1 4/4] crypto/ccp: Initialize data during __sev_snp_init_locked()

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

Sashiko notes:

> is the stack variable data left uninitialized when taking the else branch?
> Since data.tio_en is later evaluated unconditionally, could stack garbage
> cause it to evaluate to true, leading to erroneous attempts to allocate
> pages and initialize SEV-TIO on unsupported hardware?

If the firmware is too old to support SEV_INIT_EX, data is left
uninitialized but used in the debug logging about whether TIO is enabled or
not.

Fixes: 4be423572da1 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Reported-by: Sashiko
Assisted-by: Gemini:gemini-3.1-pro-preview
Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 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 11e2c667c0ad..7b8c1b44f2da 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1356,7 +1356,7 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 {
 	struct sev_data_range_list *snp_range_list __free(kfree) = NULL;
 	struct psp_device *psp = psp_master;
-	struct sev_data_snp_init_ex data;
+	struct sev_data_snp_init_ex data = {};
 	struct sev_device *sev;
 	void *arg = &data;
 	int cmd, rc = 0;
@@ -1420,8 +1420,6 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 		 */
 		snp_add_hv_fixed_pages(sev, snp_range_list);
 
-		memset(&data, 0, sizeof(data));
-
 		if (max_snp_asid) {
 			data.ciphertext_hiding_en = 1;
 			data.max_snp_asid = max_snp_asid;
-- 
2.53.0