Re: [PATCH v5 00/10] EFI Specific Purpose Memory Support
On Fri, 6 Sep 2019 13:37:30 +0200
"Rafael J. Wysocki" wrote:
> On 9/5/2019 1:06 AM, Dan Williams wrote:
> > On Mon, Sep 2, 2019 at 4:09 AM Rafael J. Wysocki
> > wrote:
> >> On Friday, August 30, 2019 3:52:18 AM CEST Dan Williams wrote:
> >>> Changes since v4 [1]:
> >>> - Rename the facility from "Application Reserved" to "Soft Reserved" to
> >>>better reflect how the memory is treated. While the spec talks about
> >>>"specific / application purpose" memory the expected kernel behavior is
> >>>to make a best effort at reserving the memory from general purpose
> >>>allocations.
> >>>
> >>> - Add a new efi=nosoftreserve option to disable consideration of the
> >>>EFI_MEMORY_SP attribute at boot time. This is also motivated by
> >>>Christoph's initial feedback of allowing the kernel to opt-out of the
> >>>policy whims of the platform BIOS implementation.
> >>>
> >>> - Update the KASLR implementation to exclude soft-reserved memory
> >>>including the case where soft-reserved memory is specified via the
> >>>efi_fake_mem= attribute-override command-line option.
> >>>
> >>> - Move the memregion allocator to its own object file. v4 had it in
> >>>kernel/resource.c which caused compile errors on Sparc. I otherwise
> >>>could not find an appropriate place to stash it.
> >>>
> >>> - Rebase on a merge of tip/master and rafael/linux-next since the series
> >>>collides with changes in both those trees.
> >>>
> >>> [1]:
> >>> https://lore.kernel.org/r/156140036490.2951909.1837804994781523185.st...@dwillia2-desk3.amr.corp.intel.com/
> >>>
> >>> ---
> >>>
> >>> Thomas, Rafael,
> >>>
> >>> This happens to collide with both your trees. I think the content
> >>> warrants going through the x86 tree, but would need to publish commit:
> >>>
> >>> 5c7ed4385424 HMAT: Skip publishing target info for nodes with no online
> >>> memory
> >>>
> >>> ...in Rafael's tree as a stable id for -tip to pull in, but I'm also
> >>> open to other options. I've retained Dave's reviewed-by from v4.
> >>>
> >>> ---
> >>>
> >>> The EFI 2.8 Specification [2] introduces the EFI_MEMORY_SP ("specific
> >>> purpose") memory attribute. This attribute bit replaces the deprecated
> >>> ACPI HMAT "reservation hint" that was introduced in ACPI 6.2 and removed
> >>> in ACPI 6.3.
> >>>
> >>> Given the increasing diversity of memory types that might be advertised
> >>> to the operating system, there is a need for platform firmware to hint
> >>> which memory ranges are free for the OS to use as general purpose memory
> >>> and which ranges are intended for application specific usage. For
> >>> example, an application with prior knowledge of the platform may expect
> >>> to be able to exclusively allocate a precious / limited pool of high
> >>> bandwidth memory. Alternatively, for the general purpose case, the
> >>> operating system may want to make the memory available on a best effort
> >>> basis as a unique numa-node with performance properties by the new
> >>> CONFIG_HMEM_REPORTING [3] facility.
> >>>
> >>> In support of optionally allowing either application-exclusive and
> >>> core-kernel-mm managed access to differentiated memory, claim
> >>> EFI_MEMORY_SP ranges for exposure as "soft reserved" and assigned to a
> >>> device-dax instance by default. Such instances can be directly owned /
> >>> mapped by a platform-topology-aware application. Alternatively, with the
> >>> new kmem facility [4], the administrator has the option to instead
> >>> designate that those memory ranges be hot-added to the core-kernel-mm as
> >>> a unique memory numa-node. In short, allow for the decision about what
> >>> software agent manages soft-reserved memory to be made at runtime.
> >>>
> >>> The patches build on the new HMAT+HMEM_REPORTING facilities merged
> >>> for v5.2-rc1. The implementation is tested with qemu emulation of HMAT
> >>> [5] plus the efi_fake_mem facility for applying the EFI_MEMORY_SP
> >>> attribute. Specific details on reproducing the test configuration are in
> >>> patch 10.
> >>>
> >>> [2]:
> >>> https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf
> >>> [3]:
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e1cf33aafb84
> >>> [4]:
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c221c0b0308f
> >>> [5]: http://patchwork.ozlabs.org/cover/1096737/
> >>>
> >>> ---
> >>>
> >>> Dan Williams (10):
> >>>acpi/numa: Establish a new drivers/acpi/numa/ directory
> >>>efi: Enumerate EFI_MEMORY_SP
> >>>x86, efi: Push EFI_MEMMAP check into leaf routines
> >>>x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
> >>>x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
> >>>lib: Uplevel the pmem "region" ida to a global allocator
> >>>dax: Fix alloc_dax_region() compile warning
> >>>device-dax: Add a driver for "hmem" devices
> >>>acpi/numa/hmat:
Re: [PATCH] efi/efi_test: require CAP_SYS_ADMIN to open the chardev
On 10/03/19 12:07, Javier Martinez Canillas wrote:
> The driver exposes EFI runtime services to user-space through an IOCTL
> interface, calling the EFI services function pointers directly without
> using the efivar API.
>
> Among other things the driver allows to read and write EFI variables and
> doesn't require CAP_SYS_ADMIN as is the case for the efivar sysfs driver.
>
> To make sure that unprivileged users won't be able to access the exposed
> EFI runtime services require CAP_SYS_ADMIN to open the character device,
> instead of just relying on the chardev file mode bits to prevent this.
>
> The main user of this driver is the fwts [0] tool, that already checks if
> the effective user ID is 0 and fails otherwise. So adding the requirement
> won't cause any regression to this tool.
>
> [0]: https://wiki.ubuntu.com/FirmwareTestSuite/Reference/uefivarinfo
>
> Signed-off-by: Javier Martinez Canillas
>
>
> ---
>
> Hello,
>
> We want to enable this driver in the Fedora kernel for testing purposes.
>
> Currently the GetVariable() UEFI runtime service is used (through the
> efivar sysfs interface) to test that OVMF is able to enter into SMM.
>
> But there's a proposal to add a UEFI variable cache outside of SMM, to
> speedup GetVariable() calls. So the plan is to call QueryVariableInfo()
> instead that's also read-only and sufficiently infrequently called that
> is not planned to be cached anytime soon.
>
> Building the efi_test module will allow us to call this EFI service by
> using the fwts uefivarinfo test. But we are worried that enabling this
> driver could open a new attack vector and lead to unprivileged users
> accessing the exposed EFI services.
>
> This is also consistent with the efivar driver since it also requires
> the CAP_SYS_ADMIN capability.
>
> Best regards,
> Javier
>
> drivers/firmware/efi/test/efi_test.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/firmware/efi/test/efi_test.c
> b/drivers/firmware/efi/test/efi_test.c
> index 877745c3aaf..81de7374c42 100644
> --- a/drivers/firmware/efi/test/efi_test.c
> +++ b/drivers/firmware/efi/test/efi_test.c
> @@ -717,6 +717,8 @@ static long efi_test_ioctl(struct file *file, unsigned
> int cmd,
>
> static int efi_test_open(struct inode *inode, struct file *file)
> {
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> /*
>* nothing special to do here
>* We do accept multiple open files at the same time as we
>
Looks consistent with other capable(CAP_SYS_ADMIN) checks in
drivers/firmware/efi/.
Acked-by: Laszlo Ersek
Thanks!
Laszlo
[PATCH] efi/efi_test: require CAP_SYS_ADMIN to open the chardev
The driver exposes EFI runtime services to user-space through an IOCTL
interface, calling the EFI services function pointers directly without
using the efivar API.
Among other things the driver allows to read and write EFI variables and
doesn't require CAP_SYS_ADMIN as is the case for the efivar sysfs driver.
To make sure that unprivileged users won't be able to access the exposed
EFI runtime services require CAP_SYS_ADMIN to open the character device,
instead of just relying on the chardev file mode bits to prevent this.
The main user of this driver is the fwts [0] tool, that already checks if
the effective user ID is 0 and fails otherwise. So adding the requirement
won't cause any regression to this tool.
[0]: https://wiki.ubuntu.com/FirmwareTestSuite/Reference/uefivarinfo
Signed-off-by: Javier Martinez Canillas
---
Hello,
We want to enable this driver in the Fedora kernel for testing purposes.
Currently the GetVariable() UEFI runtime service is used (through the
efivar sysfs interface) to test that OVMF is able to enter into SMM.
But there's a proposal to add a UEFI variable cache outside of SMM, to
speedup GetVariable() calls. So the plan is to call QueryVariableInfo()
instead that's also read-only and sufficiently infrequently called that
is not planned to be cached anytime soon.
Building the efi_test module will allow us to call this EFI service by
using the fwts uefivarinfo test. But we are worried that enabling this
driver could open a new attack vector and lead to unprivileged users
accessing the exposed EFI services.
This is also consistent with the efivar driver since it also requires
the CAP_SYS_ADMIN capability.
Best regards,
Javier
drivers/firmware/efi/test/efi_test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/firmware/efi/test/efi_test.c
b/drivers/firmware/efi/test/efi_test.c
index 877745c3aaf..81de7374c42 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -717,6 +717,8 @@ static long efi_test_ioctl(struct file *file, unsigned int
cmd,
static int efi_test_open(struct inode *inode, struct file *file)
{
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
/*
* nothing special to do here
* We do accept multiple open files at the same time as we
--
2.21.0
