linux-kernel - Re: [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240425133024.GIZipa8F1tkFGXuNNZ@fat_crate.local>
Date: Thu, 25 Apr 2024 15:30:24 +0200
From: Borislav Petkov <bp@...en8.de>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
	linux-coco@...ts.linux.dev, svsm-devel@...onut-svsm.dev,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Dan Williams <dan.j.williams@...el.com>,
	Michael Roth <michael.roth@....com>,
	Ashish Kalra <ashish.kalra@....com>
Subject: Re: [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to
 snp_secrets_page

On Wed, Apr 24, 2024 at 10:57:57AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 995f94467101..6949fbccec40 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -648,7 +648,7 @@ static u64 __init get_secrets_page(void)
>  
>  static u64 __init get_snp_jump_table_addr(void)
>  {
> -	struct snp_secrets_page_layout *layout;
> +	struct snp_secrets_page *layout;

Yes, and I'd go change that "layout" name to "secrets" too because
layout doesn't make any sense when talking about a secrets page.

This, OTOH:

	addr = secrets->os_area.ap_jump_table_pa;

means something: the address comes from the secrets page. Not from the
"layout". :-)

IOW, diff ontop:

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 25056346bc18..790e4818f7c6 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -648,7 +648,7 @@ static u64 __init get_secrets_page(void)
 
 static u64 __init get_snp_jump_table_addr(void)
 {
-	struct snp_secrets_page *layout;
+	struct snp_secrets_page *secrets;
 	void __iomem *mem;
 	u64 pa, addr;
 
@@ -662,9 +662,9 @@ static u64 __init get_snp_jump_table_addr(void)
 		return 0;
 	}
 
-	layout = (__force struct snp_secrets_page *)mem;
+	secrets = (__force struct snp_secrets_page *)mem;
 
-	addr = layout->os_area.ap_jump_table_pa;
+	addr = secrets->os_area.ap_jump_table_pa;
 	iounmap(mem);
 
 	return addr;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 04a7bd1e4314..654290a8e1ba 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -59,7 +59,7 @@ struct snp_guest_dev {
 	 */
 	struct snp_guest_msg secret_request, secret_response;
 
-	struct snp_secrets_page *layout;
+	struct snp_secrets_page *secrets;
 	struct snp_req_data input;
 	union {
 		struct snp_report_req report;
@@ -743,26 +743,26 @@ static const struct file_operations snp_guest_fops = {
 	.unlocked_ioctl = snp_guest_ioctl,
 };
 
-static u8 *get_vmpck(int id, struct snp_secrets_page *layout, u32 **seqno)
+static u8 *get_vmpck(int id, struct snp_secrets_page *secrets, u32 **seqno)
 {
 	u8 *key = NULL;
 
 	switch (id) {
 	case 0:
-		*seqno = &layout->os_area.msg_seqno_0;
-		key = layout->vmpck0;
+		*seqno = &secrets->os_area.msg_seqno_0;
+		key = secrets->vmpck0;
 		break;
 	case 1:
-		*seqno = &layout->os_area.msg_seqno_1;
-		key = layout->vmpck1;
+		*seqno = &secrets->os_area.msg_seqno_1;
+		key = secrets->vmpck1;
 		break;
 	case 2:
-		*seqno = &layout->os_area.msg_seqno_2;
-		key = layout->vmpck2;
+		*seqno = &secrets->os_area.msg_seqno_2;
+		key = secrets->vmpck2;
 		break;
 	case 3:
-		*seqno = &layout->os_area.msg_seqno_3;
-		key = layout->vmpck3;
+		*seqno = &secrets->os_area.msg_seqno_3;
+		key = secrets->vmpck3;
 		break;
 	default:
 		break;
@@ -898,7 +898,7 @@ static void unregister_sev_tsm(void *data)
 static int __init sev_guest_probe(struct platform_device *pdev)
 {
 	struct sev_guest_platform_data *data;
-	struct snp_secrets_page *layout;
+	struct snp_secrets_page *secrets;
 	struct device *dev = &pdev->dev;
 	struct snp_guest_dev *snp_dev;
 	struct miscdevice *misc;
@@ -916,7 +916,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 	if (!mapping)
 		return -ENODEV;
 
-	layout = (__force void *)mapping;
+	secrets = (__force void *)mapping;
 
 	ret = -ENOMEM;
 	snp_dev = devm_kzalloc(&pdev->dev, sizeof(struct snp_guest_dev), GFP_KERNEL);
@@ -924,7 +924,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 		goto e_unmap;
 
 	ret = -EINVAL;
-	snp_dev->vmpck = get_vmpck(vmpck_id, layout, &snp_dev->os_area_msg_seqno);
+	snp_dev->vmpck = get_vmpck(vmpck_id, secrets, &snp_dev->os_area_msg_seqno);
 	if (!snp_dev->vmpck) {
 		dev_err(dev, "invalid vmpck id %d\n", vmpck_id);
 		goto e_unmap;
@@ -938,7 +938,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, snp_dev);
 	snp_dev->dev = dev;
-	snp_dev->layout = layout;
+	snp_dev->secrets = secrets;
 
 	/* Allocate the shared page used for the request and response message. */
 	snp_dev->request = alloc_shared_pages(dev, sizeof(struct snp_guest_msg));

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette