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

Naveen N Rao
2025-04-07T13:16:01Z

On Thu, Apr 03, 2025 at 12:06:29PM -0700, Dan Williams wrote:
> Naveen N Rao (AMD) wrote:
> > Commit 9704c07bf9f7 ("x86/kernel: Validate ROM memory before accessing
> > when SEV-SNP is active") added code to validate the ROM region from
> > 0xc0000 to 0xfffff in a SEV-SNP guest since that region can be accessed
> > during kernel boot. That address range is not part of the system RAM, so
> > it needed to be validated separately.
> >
> > Commit 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> > SEV-SNP guests") reverted those changes and instead chose to prevent the
> > guest from accessing the ROM region since SEV-SNP guests did not rely on
> > data from that region. However, while the kernel itself no longer
> > accessed the ROM region, there are userspace programs that probe this
> > region through /dev/mem and they started crashing due to this change. In
> > particular, fwupd (up until versions released last year that no longer
> > link against libsmbios) and smbios utilities such as smbios-sys-info
> > crash with a cryptic message in dmesg:
> > Wrong/unhandled opcode bytes: 0x8b, exit_code: 0x404, rIP: 0x7fe5404d3840
> > SEV: Unsupported exit-code 0x404 in #VC exception (IP: 0x7fe5404d3840)
> >
> > Deny access to the BIOS region (rather than just the video ROM range)
> > via /dev/mem to address this. Restrict changes to CONFIG_STRICT_DEVMEM=y
> > which is enabled by default on x86. Add a new x86_platform_ops callback
> > so Intel can customize the address range to block.
> >
> > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> > ---
> > arch/x86/coco/sev/core.c | 13 +++++++++++++
> > arch/x86/include/asm/sev.h | 2 ++
> > arch/x86/include/asm/x86_init.h | 2 ++
> > arch/x86/kernel/x86_init.c | 2 ++
> > arch/x86/mm/init.c | 3 +++
> > arch/x86/mm/mem_encrypt_amd.c | 1 +
> > 6 files changed, 23 insertions(+)
> >
<snip>
>
> Is there any driving need to allow devmem at all for TVM access at this
> point?
>
> I would be in favor of making this clearly tied to devmem, call it
> ".devmem_is_allowed" for symmetry with the mm/init.c helper, and make
> the default implementation be:
>
> static bool platform_devmem_is_allowed(unsigned long pfn)
> {
> return !cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
> }
>
> ...if a TVM technology wants more leniency, it can override.

I'm not fully aware of the history here, but I suppose a TVM should
appear as any other VM for userspace. For that reason, I didn't want to
block access to /dev/mem any more than was necessary. Admittedly, I have
limited insight into which utilities may be using /dev/mem today.

Tom/Boris, do you see a problem blocking access to /dev/mem for SEV
guests?

- Naveen