[PATCH 1/3] drm/amdgpu: Add flag for allocating memory for sensitive data

[Michel Dänzer]

On 2019-07-09 9:00 p.m., Kuehling, Felix wrote:
> On 2019-07-09 6:34 a.m., Michel Dänzer wrote:
>> On 2019-07-09 7:32 a.m., Kuehling, Felix wrote:
>>> This memory allocation flag will be used to indicate BOs containing
>>> sensitive data that should not be leaked to other processes.
>>>
>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>> ---
>>>   include/uapi/drm/amdgpu_drm.h | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>>> index 61870478bc9c..58659c28c26e 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -131,6 +131,10 @@ extern "C" {
>>>    * for the second page onward should be set to NC.
>>>    */
>>>   #define AMDGPU_GEM_CREATE_MQD_GFX9		(1 << 8)
>>> +/* Flag that BO may contain sensitive data that must be cleared before
>>> + * releasing the memory
>>> + */
>>> +#define AMDGPU_GEM_CREATE_VRAM_SENSITIVE	(1 << 9)
>>>   
>>>   struct drm_amdgpu_gem_create_in  {
>>>   	/** the requested memory size */
>>>
>> This flag essentially means "Please don't leak my BO contents".
>> Similarly, AMDGPU_GEM_CREATE_VRAM_CLEARED essentially means "Please
>> don't let me see previous memory contents".
>>
>> I'd argue that neither flag should really be needed; BO contents
>> shouldn't be leaked by default.
> 
> My conclusion from previous discussions was that CREATE_VRAM_CLEARED has 
> no security implications. It's basically completely ineffective as a 
> security measure.

Absolutely, which is why I argued against it when it was proposed.

> It's more a convenience feature. Therefore I think it still has a place
> as that.

It'd be a no-op if memory was always cleared. :)


> I'd agree on principle that data shouldn't be leaked by default, but it 
> has been the default for a long time. My impression was that graphics 
> guys cared more about performance than security. So changing the default 
> may be a hard sell. On the compute side we already took a big 
> performance hit by clearing all our VRAM, so this change would be an 
> improvement for us. Therefore I think it still makes sense to let the 
> application choose.

What exactly could userspace be allowed to choose though? I can only
think of disabling the clearing of memory it allocates ("Please leak my
BO contents"), which seems of rather dubious value.

What might be feasible is allowing the system administrator to disable
it, similar to the mitigations for Meltdown/Spectre/... vulnerabilities.
OTOH I don't know of any mechanism for disabling the clearing of normal
system RAM (which is also effective at least to some degree for BOs
outside of VRAM, making the whole thing a bit inconsistent).


-- 
Earthling Michel Dänzer               |              https://www.amd.com
Libre software enthusiast             |             Mesa and X developer