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

[Tom Lendacky]

On 5/20/26 21:10, Pratik R. Sampat wrote:
> 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@xxxxxxx>
>>> ---
>>>  .../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@xxxxxxxxxxxxxxx
>>> +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:

Can it? Can init and shutdown race? Isn't that part of module load /
unload, I'm not sure how they can race...

> 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.

I don't think you can have init vs init race, can you? This just all seems
odd to me. Have you created all these race scenarios to test this out?

Would putting the regsiter/unregister under the sev_cmd_mutex and then
taking the sev_cmd_mutex upon entry to _show()/_store() fix all this?
After obtaining the mutex in _show()/_store(), you check for
sev->verify_mit and return an error if NULL. Then you can use the
__sev_do_cmd_locked() to issue any commands.

Also, on the register function, all you need is the check for
!(sev->snp_feat_info_0.ecx & SNP_VERIFY_MITIGATION_SUPPORTED) since if
!sev->snp_plat_status.feature_info is true, so is this this check. And, as
the spec says, the required firmware state is based on the mitigation
requirements, so I don't think you should be checking for snp_initialized.

Thanks,
Tom

> 
> So, I attempt address that with a sev_mit_sysfs_mutex guard.
> 
> --Pratik