linux-kernel - [PATCH][v2] virt/coco/sev-guest: Don't free decrypted memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20240619092215.1824-1-lirongqing@baidu.com>
Date: Wed, 19 Jun 2024 17:22:15 +0800
From: Li RongQing <lirongqing@...du.com>
To: thomas.lendacky@....com,
	dan.j.williams@...el.com,
	bp@...en8.de,
	linux-kernel@...r.kernel.org
Cc: Li RongQing <lirongqing@...du.com>
Subject: [PATCH][v2] virt/coco/sev-guest: Don't free decrypted memory

If set_memory_decrypted() fails, and memory maybe have a mix of
pagetable entries, that could be a problem.

As Tom explained:
  As long as the encryption bit hasn't been cleared in any of the
  guest pagetables for the page range, then there should not be an
  issue. When the page is referenced it will generate a #NPF and
  the host will have to make that page a private page in order for
  forward progress to be made. But, that page will already have
  been PVALIDATEd previously, so the resulting #VC for the page no
  longer being PVALIDATEd will allow the guest to detect the
  malicious hypervisor and terminate.

  If we fail during the __change_page_attr_set_clr() call and we get
  a mix of pagetable entries that could be a problem, so leaking the
  pages would be best in that case.

  And since the failure reason isn't clear after the call, leaking
  the pages is probably the safest thing.

Suggested-by: Tom Lendacky <thomas.lendacky@....com>
Signed-off-by: Li RongQing <lirongqing@...du.com>
---
diff with v1: update the commit log

 drivers/virt/coco/sev-guest/sev-guest.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 654290a8e..d31f229 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -730,8 +730,7 @@ static void *alloc_shared_pages(struct device *dev, size_t sz)

 	ret = set_memory_decrypted((unsigned long)page_address(page), npages);
 	if (ret) {
-		dev_err(dev, "failed to mark page shared, ret=%d\n", ret);
-		__free_pages(page, get_order(sz));
+		dev_err(dev, "failed to mark page shared, leak it, ret=%d\n", ret);
 		return NULL;
 	}

-- 
2.9.4