Re: [PATCH v3] crypto/ccp: Introduce SNP_VERIFY_MITIGATION command

["Pratik R. Sampat" <prsampat@amd.com>]

Hi Tom,

On 5/20/26 4:22 PM, Tom Lendacky wrote:
> On 5/19/26 14:50, Pratik R. Sampat wrote:
>> The SEV-SNP firmware provides the SNP_VERIFY_MITIGATION command, which
>> can be used to query the status of currently supported vulnerability
>> mitigations and to initiate mitigations within the firmware.
>>
>> This command is an explicit mechanism to ascertain if a firmware
>> mitigation is applied without needing a full RMP re-build, which is most
>> useful in a live firmware update scenario.
>>
>> The firmware supports two subcommands: STATUS and VERIFY. The STATUS
>> subcommand is used to query the supported and verified mitigation bits.
>> The VERIFY subcommand initiates the mitigation process within the FW for
>> the specified vulnerability. Expose a userspace interface under:
>> /sys/firmware/sev/vulnerabilities/
>>   - supported_mitigations (read-only): supported mitigation vector mask
>>   - verified_mitigations (read/write): current verified mask; write a
>>     vector to request VERIFY for that bit
>>
>> The behavior of SNP_VERIFY_MITIGATION and the pre-requisites for using
>> it are bug-specific. Information about supported mitigations and its
>> corresponding vector is to be published as part of the AMD Security
>> Bulletin.
>>
>> See SEV-SNP Firmware ABI specifications 1.58, SNP_VERIFY_MITIGATION for
>> more details.
>>
>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>> ---
>>  .../sysfs-firmware-sev-vulnerabilities        |  17 ++
>>  drivers/crypto/ccp/sev-dev.c                  | 172 ++++++++++++++++++
>>  drivers/crypto/ccp/sev-dev.h                  |   3 +
>>  include/linux/psp-sev.h                       |  51 ++++++
>>  4 files changed, 243 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-sev-vulnerabilities
>>
>> diff --git a/Documentation/ABI/testing/sysfs-firmware-sev-vulnerabilities b/Documentation/ABI/testing/sysfs-firmware-sev-vulnerabilities
>> new file mode 100644
>> index 000000000000..cc84adbac3c0
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-firmware-sev-vulnerabilities
>> @@ -0,0 +1,17 @@
>> +What:		/sys/firmware/sev/vulnerabilities/
>> +		/sys/firmware/sev/vulnerabilities/supported_mitigations
>> +		/sys/firmware/sev/vulnerabilities/verified_mitigations
>> +Date:		May 2026
>> +Contact:	linux-crypto@vger.kernel.org
>> +Description:	Information about SEV-SNP firmware vulnerability mitigations.
>> +		supported_mitigations: Read-only interface that reports
>> +				       the vector of mitigations supported by
>> +				       the firmware.
>> +		verified_mitigations: Read/write interface that reports
>> +				      the vector of mitigations already verified
>> +				      by the firmware. Writing a vector value
>> +				      requests the firmware to VERIFY the
>> +				      corresponding mitigation bit(s).
>> +		The list of supported mitigations and the meaning of each
>> +		vector bit are both platform- and bug-specific and are
>> +		published as part of the AMD Security Bulletin.
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index d1e9e0ac63b6..eec4864c6597 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -57,6 +57,7 @@
>>  #define CMD_BUF_DESC_MAX (CMD_BUF_FW_WRITABLE_MAX + 1)
>>  
>>  static DEFINE_MUTEX(sev_cmd_mutex);
>> +static DEFINE_MUTEX(sev_mit_sysfs_mutex);
>>  static struct sev_misc_dev *misc_dev;
>>  
>>  static int psp_cmd_timeout = 100;
>> @@ -245,6 +246,7 @@ static int sev_cmd_buffer_len(int cmd)
>>  	case SEV_CMD_SNP_LAUNCH_FINISH:		return sizeof(struct sev_data_snp_launch_finish);
>>  	case SEV_CMD_SNP_DBG_DECRYPT:		return sizeof(struct sev_data_snp_dbg);
>>  	case SEV_CMD_SNP_DBG_ENCRYPT:		return sizeof(struct sev_data_snp_dbg);
>> +	case SEV_CMD_SNP_VERIFY_MITIGATION:	return sizeof(struct sev_data_snp_verify_mitigation);
>>  	case SEV_CMD_SNP_PAGE_UNSMASH:		return sizeof(struct sev_data_snp_page_unsmash);
>>  	case SEV_CMD_SNP_PLATFORM_STATUS:	return sizeof(struct sev_data_snp_addr);
>>  	case SEV_CMD_SNP_GUEST_REQUEST:		return sizeof(struct sev_data_snp_guest_request);
>> @@ -1351,6 +1353,162 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>>  	return 0;
>>  }
>>  
>> +static int snp_verify_mitigation(u16 command, u64 vector,
>> +				 struct sev_data_snp_verify_mitigation_dst *dst)
>> +{
>> +	struct sev_data_snp_verify_mitigation_dst *mit_dst = NULL;
>> +	struct sev_data_snp_verify_mitigation data = {0};
>> +	struct sev_device *sev = psp_master->sev_data;
>> +	int ret, error = 0;
>> +
>> +	mit_dst = snp_alloc_firmware_page(GFP_KERNEL | __GFP_ZERO);
>> +	if (!mit_dst)
>> +		return -ENOMEM;
>> +
>> +	data.length = sizeof(data);
>> +	data.subcommand = command;
>> +	data.vector = vector;
>> +	data.dst_paddr = __psp_pa(mit_dst);
>> +	data.dst_paddr_en = true;
>> +
>> +	ret = sev_do_cmd(SEV_CMD_SNP_VERIFY_MITIGATION, &data, &error);
>> +	if (!ret)
>> +		memcpy(dst, mit_dst, sizeof(*mit_dst));
>> +	else
>> +		dev_err(sev->dev, "SNP_VERIFY_MITIGATION command failed, ret = %d, error = %#x\n",
>> +			ret, error);
>> +
>> +	snp_free_firmware_page(mit_dst);
>> +
>> +	return ret;
>> +}
> 
> Should this function also be under the CONFIG_SYSFS #ifdef? Won't you get
> an unused function warning if CONFIG_SYSFS isn't defined?

That's right. Thanks for spotting that!

> 
>> +
>> +#ifdef CONFIG_SYSFS
>> +static ssize_t supported_mitigations_show(struct kobject *kobj,
>> +					  struct kobj_attribute *attr, char *buf)
>> +{
>> +	struct sev_data_snp_verify_mitigation_dst dst;
>> +	int ret;
>> +
>> +	ret = snp_verify_mitigation(SNP_MIT_SUBCMD_REQ_STATUS, 0, &dst);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "0x%llx\n", dst.mit_supported_vector);
>> +}
>> +
>> +static struct kobj_attribute supported_attr =
>> +		__ATTR_RO_MODE(supported_mitigations, 0400);
>> +
>> +static ssize_t verified_mitigations_show(struct kobject *kobj,
>> +					 struct kobj_attribute *attr, char *buf)
>> +{
>> +	struct sev_data_snp_verify_mitigation_dst dst;
>> +	int ret;
>> +
>> +	ret = snp_verify_mitigation(SNP_MIT_SUBCMD_REQ_STATUS, 0, &dst);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sysfs_emit(buf, "0x%llx\n", dst.mit_verified_vector);
>> +}
>> +
>> +static ssize_t verified_mitigations_store(struct kobject *kobj,
>> +					  struct kobj_attribute *attr,
>> +					  const char *buf, size_t count)
>> +{
>> +	struct sev_data_snp_verify_mitigation_dst dst;
>> +	struct sev_device *sev = psp_master->sev_data;
>> +	u64 vector;
>> +	int ret;
>> +
>> +	ret = kstrtoull(buf, 0, &vector);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = snp_verify_mitigation(SNP_MIT_SUBCMD_REQ_VERIFY, vector, &dst);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (dst.mit_failure_status) {
>> +		dev_err(sev->dev, "Verify Mitigation - failure status: 0x%x\n",
>> +			dst.mit_failure_status);
>> +		return -EIO;
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +static struct kobj_attribute verified_attr =
>> +		__ATTR_RW_MODE(verified_mitigations, 0600);
>> +
>> +static struct attribute *mitigation_attrs[] = {
>> +	&supported_attr.attr,
>> +	&verified_attr.attr,
>> +	NULL
>> +};
>> +
>> +static const struct attribute_group mit_attr_group = {
>> +	.attrs = mitigation_attrs,
>> +};
>> +
>> +static void sev_snp_register_verify_mitigation(struct sev_device *sev)
>> +{
>> +	int rc;
>> +
>> +	if (!sev->snp_initialized || !sev->snp_plat_status.feature_info ||
>> +	    !(sev->snp_feat_info_0.ecx & SNP_VERIFY_MITIGATION_SUPPORTED))
>> +		return;
>> +
>> +	guard(mutex)(&sev_mit_sysfs_mutex);
>> +
>> +	if (sev->verify_mit)
>> +		return;
>> +
>> +	if (!sev->sev_kobj) {
>> +		sev->sev_kobj = kobject_create_and_add("sev", firmware_kobj);
>> +		if (!sev->sev_kobj)
>> +			return;
>> +	}
>> +
>> +	sev->verify_mit = kobject_create_and_add("vulnerabilities", sev->sev_kobj);
>> +	if (!sev->verify_mit)
>> +		goto err_sev_kobj;
>> +
>> +	rc = sysfs_create_group(sev->verify_mit, &mit_attr_group);
>> +	if (rc)
>> +		goto err_verify_mit;
>> +
>> +	return;
>> +
>> +err_verify_mit:
>> +	kobject_put(sev->verify_mit);
>> +	sev->verify_mit = NULL;
>> +err_sev_kobj:
>> +	kobject_put(sev->sev_kobj);
>> +	sev->sev_kobj = NULL;
>> +}
>> +
>> +static void sev_snp_unregister_verify_mitigation(struct sev_device *sev)
>> +{
>> +	guard(mutex)(&sev_mit_sysfs_mutex);
>> +
>> +	if (sev->verify_mit) {
>> +		sysfs_remove_group(sev->verify_mit, &mit_attr_group);
>> +		kobject_put(sev->verify_mit);
>> +		sev->verify_mit = NULL;
>> +	}
>> +
>> +	if (sev->sev_kobj) {
>> +		kobject_put(sev->sev_kobj);
>> +		sev->sev_kobj = NULL;
>> +	}
>> +}
>> +#else
>> +static void sev_snp_register_verify_mitigation(struct sev_device *sev) { }
>> +static void sev_snp_unregister_verify_mitigation(struct sev_device *sev) { }
>> +#endif
>> +
>>  static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>>  {
>>  	struct sev_data_range_list *snp_range_list __free(kfree) = NULL;
>> @@ -1670,6 +1828,14 @@ int sev_platform_init(struct sev_platform_init_args *args)
>>  	rc = _sev_platform_init_locked(args);
>>  	mutex_unlock(&sev_cmd_mutex);
>>  
>> +	/*
>> +	 * The shutdown + init path can race with in-flight _show()/_store() operations
>> +	 * which acquire the sev_cmd_mutex. Register the sysfs interface outside
>> +	 * the sev_cmd_mutex and serialize by sev_mit_sysfs_mutex instead.
> 
> I'm not quite sure I follow this. The shutdown and init path can't race
> with each other, right? In which case this new mutex doesn't really matter
> unless you take it on _show()/_short(), right?
> 
What I meant here is the new mutex attempts to addresses the following scenario:

First, assume sev_snp_[un]register_verify_mitigation() are protected under
sev_cmd_mutex:

t1                                 | t2
---------------------------------- | ----------------------------------
sev_firmware_shutdown()            |
  lock(sev_cmd_mutex)              |
                                   | verified_mitigations_store()
                                   |   lock(sev_cmd_mutex)  <-- waits on t1
  unregister_verify_mitigation()   |
    sysfs_remove_group()  <-- waits for t2's _store to drain

So sev_snp_unregister_verify_mitigation() has to run outside sev_cmd_mutex to
avoid the sysfs_remove_group() <-> in-flight _show()/_store() deadlock.

Now, with unregister no longer protected by sev_cmd_mutex, a concurrent init
can race with shutdown on the sysfs lifetime like so:
t1                                 | t2
---------------------------------- | ----------------------------------
sev_firmware_shutdown()            | sev_platform_init()
  unregister_verify_mitigation()   |   register_verify_mitigation()
    sysfs_remove_group()           |     sysfs_create_group()

Both sides touch sev->verify_mit without serialization. The same race also
exists for init vs init which is no longer covered by sev_cmd_mutex once
register moves outside it.

So, I attempt address that with a sev_mit_sysfs_mutex guard.

--Pratik