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 <javierm@xxxxxxxxxx> > > > --- > > 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 <lersek@xxxxxxxxxx> Thanks! Laszlo
