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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e11e9b17-a9e0-4f3d-964f-c8b656403f65@suse.com>
Date: Thu, 10 Apr 2025 22:27:21 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: Dan Williams <dan.j.williams@...el.com>, Kees Cook <kees@...nel.org>
Cc: Dave Hansen <dave.hansen@...el.com>,
 Tom Lendacky <thomas.lendacky@....com>, Naveen N Rao <naveen@...nel.org>,
 linux-kernel@...r.kernel.org, x86@...nel.org, linux-coco@...ts.linux.dev,
 Dave Hansen <dave.hansen@...ux.intel.com>, Borislav Petkov <bp@...en8.de>,
 Vishal Annapurve <vannapurve@...gle.com>,
 Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
 Kevin Loughlin <kevinloughlin@...gle.com>, Michal Hocko <mhocko@...e.com>
Subject: Re: [RFC PATCH] x86/sev: Disallow userspace access to BIOS region for
 SEV-SNP guests



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,
> };
>