linux-kernel - Re: [PATCH v3 13/14] x86/sev: Hide SVSM attestation entries if not running under an SVSM

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]

Date: Tue, 16 Apr 2024 11:07:26 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Dan Williams <dan.j.williams@...el.com>, linux-kernel@...r.kernel.org,
 x86@...nel.org, linux-coco@...ts.linux.dev, svsm-devel@...onut-svsm.dev
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Michael Roth <michael.roth@....com>,
 Ashish Kalra <ashish.kalra@....com>, Joel Becker <jlbec@...lplan.org>,
 Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v3 13/14] x86/sev: Hide SVSM attestation entries if not
 running under an SVSM

On 4/16/24 00:47, Dan Williams wrote:
> Tom Lendacky wrote:
>> Config-fs provides support to hide individual attribute entries. Using
>> this support, base the display of the SVSM related entries on the presence
>> of an SVSM.
>>
>> Cc: Joel Becker <jlbec@...lplan.org>
>> Cc: Christoph Hellwig <hch@....de>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>>   arch/x86/coco/core.c        |  4 ++++
>>   drivers/virt/coco/tsm.c     | 14 ++++++++++----
>>   include/linux/cc_platform.h |  8 ++++++++
>>   3 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
>> index d07be9d05cd0..efa0f648f754 100644
>> --- a/arch/x86/coco/core.c
>> +++ b/arch/x86/coco/core.c
>> @@ -12,6 +12,7 @@
>>   
>>   #include <asm/coco.h>
>>   #include <asm/processor.h>
>> +#include <asm/sev.h>
>>   
>>   enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
>>   u64 cc_mask __ro_after_init;
>> @@ -78,6 +79,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)
>>   	case CC_ATTR_GUEST_STATE_ENCRYPT:
>>   		return sev_status & MSR_AMD64_SEV_ES_ENABLED;
>>   
>> +	case CC_ATTR_GUEST_SVSM_PRESENT:
>> +		return snp_get_vmpl();
>> +
>>   	/*
>>   	 * With SEV, the rep string I/O instructions need to be unrolled
>>   	 * but SEV-ES supports them through the #VC handler.
>> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
>> index 46f230bf13ac..d30471874e87 100644
>> --- a/drivers/virt/coco/tsm.c
>> +++ b/drivers/virt/coco/tsm.c
>> @@ -64,6 +64,12 @@ static struct tsm_report_state *to_state(struct tsm_report *report)
>>   	return container_of(report, struct tsm_report_state, report);
>>   }
>>   
>> +static bool provider_visibility(const struct config_item *item,
>> +				const struct configfs_attribute *attr)
>> +{
>> +	return cc_platform_has(CC_ATTR_GUEST_SVSM_PRESENT);
>> +}
>> +
>>   static int try_advance_write_generation(struct tsm_report *report)
>>   {
>>   	struct tsm_report_state *state = to_state(report);
>> @@ -144,7 +150,7 @@ static ssize_t tsm_report_service_provider_store(struct config_item *cfg,
>>   
>>   	return len;
>>   }
>> -CONFIGFS_ATTR_WO(tsm_report_, service_provider);
>> +CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, service_provider, provider_visibility);
>>   
>>   static ssize_t tsm_report_service_guid_store(struct config_item *cfg,
>>   					     const char *buf, size_t len)
>> @@ -165,7 +171,7 @@ static ssize_t tsm_report_service_guid_store(struct config_item *cfg,
>>   
>>   	return len;
>>   }
>> -CONFIGFS_ATTR_WO(tsm_report_, service_guid);
>> +CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, service_guid, provider_visibility);
>>   
>>   static ssize_t tsm_report_service_manifest_version_store(struct config_item *cfg,
>>   							 const char *buf, size_t len)
>> @@ -186,7 +192,7 @@ static ssize_t tsm_report_service_manifest_version_store(struct config_item *cfg
>>   
>>   	return len;
>>   }
>> -CONFIGFS_ATTR_WO(tsm_report_, service_manifest_version);
>> +CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, service_manifest_version, provider_visibility);
>>   
>>   static ssize_t tsm_report_inblob_write(struct config_item *cfg,
>>   				       const void *buf, size_t count)
>> @@ -333,7 +339,7 @@ static ssize_t tsm_report_manifestblob_read(struct config_item *cfg, void *buf,
>>   
>>   	return tsm_report_read(report, buf, count, TSM_MANIFEST);
>>   }
>> -CONFIGFS_BIN_ATTR_RO(tsm_report_, manifestblob, NULL, TSM_OUTBLOB_MAX);
>> +CONFIGFS_BIN_ATTR_VISIBLE_RO(tsm_report_, manifestblob, NULL, TSM_OUTBLOB_MAX, provider_visibility);
> 
> Yeah the same callback specified multiple times feels like something
> that should only happen once at the group level.

I went with this to keep compatibility of all the attributes existing at 
the same level in the report sub-directory. Moving the existing extra 
attributes will break backwards compatibility if we want to apply 
visibility to them.

If we don't want to apply visibility to the existing extra attributes, 
then, yes, the the visibility support can be done at the group level 
instead of the file/attribute level.

Thoughts?

Thanks,
Tom

>