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 <[email protected]>
---
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
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 <[email protected]>
>
>
> ---
>
> 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 <[email protected]>
Thanks!
Laszlo