Re: [RFC PATCH] x86/sev: Disallow userspace access to BIOS region for SEV-SNP guests

Nikolay Borisov
2025-04-10T19:27:25Z

On 10.04.25 г. 22:20 ч., Dan Williams wrote:
> Nikolay Borisov wrote:
> [..]
>>>> Can't we simply treat return value of 2 for range_is_allowed the same way as
>>>> if 0 was returned in mmap_mem and simply fail the call with -EPERM?
>>>
>>> The historical concern was that EPERM would break old tools. I don't
>>> have any current evidence either way, though.
>>
>> Right, but we are only about to return 2 in a TVM context, so chances of
>> running old tools are slim to none. Also it's perfectly valid to have
>> mmap fail for a number of reasons, so old tools should be equipped with
>> handling it returning -EPERM, no ?
>
> In practice that is yet another return code since the caller does not
> know why the "2" is being returned and it is not clear how safe it is to
> now start denying mmap in the !TVM case. So, perhaps something like this:
>

What I meant by "returning 2" is returning 2 from the call to
range_is_allowed in mmap_mem and handling this value inside mmap_mem,
not returning 2 to user space :) In essence something along the lines of:

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 169eed162a7f..8273066b6637 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -359,7 +359,8 @@ static int mmap_mem(struct file *file, struct
vm_area_struct *vma)
if (!private_mapping_ok(vma))
return -ENOSYS;

- if (!range_is_allowed(vma->vm_pgoff, size))
+ int ret = range_is_allowed(vma->vm_pgoff, size);
+ if (!ret || ret == 2)
return -EPERM;

if (!phys_mem_access_prot_allowed(file, vma->vm_pgoff, size,

> enum devmem_policy {
> DEVMEM_DENY,
> DEVMEM_ALLOW,
> DEVMEM_ZERO_RW, /* XXX: fix mmap_mem to install zeroes? */
> DEVMEM_ZERO_RW_DENY_MMAP,
> };
>
> The hope is that legacy tools are either fine with open() failures due
> to the prevalance of lockdown, fine with read/write of zeroes to BIOS
> data due to the prevalance of CONFIG_STRICT_DEVMEM, or otherwise would
> not notice / break when mmap() starts failing for BIOS data in the TVM
> case. The !TVM case continues with the current gap for mmap.
>
> Or, rip the bandaid and do this to see who screams:
>
> enum devmem_policy {
> DEVMEM_DENY,
> DEVMEM_ALLOW,
> DEVMEM_ZERO_RW_DENY_MMAP,
> };
>