Re: [PATCH v5 11/17] platform/x86/amd/pmf: Add capability to sideload of policy binary

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

Hi Ilpo,

On 11/21/2023 5:47 PM, Ilpo Järvinen wrote:
> On Fri, 17 Nov 2023, Shyam Sundar S K wrote:
> 
>> A policy binary is OS agnostic, and the same policies are expected to work
>> across the OSes.  At times it becomes difficult to debug when the policies
>> inside the policy binaries starts to misbehave. Add a way to sideload such
>> policies independently to debug them via a debugfs entry.
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
> 
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 5f10e5c6335e..f73663c629fe 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> 
>> +#ifdef CONFIG_AMD_PMF_DEBUG
>> +static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
>> +				   size_t length, loff_t *pos)
>> +{
>> +	struct amd_pmf_dev *dev = filp->private_data;
>> +	int ret;
>> +
>> +	/* Policy binary size cannot exceed POLICY_BUF_MAX_SZ */
>> +	if (length > POLICY_BUF_MAX_SZ || length == 0)
>> +		return -EINVAL;
>> +
>> +	dev->policy_sz = length;
>> +	if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
>> +		return -EFAULT;
>> +
>> +	ret = amd_pmf_start_policy_engine(dev);
> 
> Is this call safe against concurrent invocations from two racing writes?
> 
> Other than that, this change looked fine.


This path gets enabled only when CONFIG_AMD_PMF_DEBUG option is
enabled. Also when enabled, did not observe anything really unusual
(like races). So I have retained the same code in v6.

Kindly take a look.

Thanks,
Shyam

> 
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	return length;
>> +}
> 
>