Re: [dpdk-dev] [PATCH] igb_uio: fail and log if kernel lock down is enabled

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/15/2018 6:47 PM, Luca Boccassi wrote:
> On Tue, 2018-05-15 at 17:56 +0100, Ferruh Yigit wrote:
>> When EFI secure boot is enabled, it is possible to lock down kernel
>> and
>> prevent accessing device BARs and this makes igb_uio unusable.
>>
>> Lock down patches are not part of the vanilla kernel but they are
>> applied and used by some distros already [1].
>>
>> It is not possible to fix this issue, but intention of this patch is
>> to
>> detect and log if kernel lock down enabled and don't insert the
>> module
>> for that case.
>>
>> The challenge is since this feature enabled by distros, they have
>> different config options and APIs for it. This patch is done based on
>> Fedora and Ubuntu kernel source, may needs to add more distro
>> specific
>> support.
>>
>> [1]
>> kernel.ubuntu.com/git/ubuntu/ubuntu-
>> artful.git/commit/?id=99f9ef18d5b6
>> And a few more patches to
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> Cc: Luca Boccassi <bluca@debian.org>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Cc: Neil Horman <nhorman@tuxdriver.com>
>> Cc: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>>  kernel/linux/igb_uio/compat.h  | 24 ++++++++++++++++++++----
>>  kernel/linux/igb_uio/igb_uio.c |  5 +++++
>>  2 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/linux/igb_uio/compat.h
>> b/kernel/linux/igb_uio/compat.h
>> index d9f4d29fc..774c980c2 100644
>> --- a/kernel/linux/igb_uio/compat.h
>> +++ b/kernel/linux/igb_uio/compat.h
>> @@ -125,10 +125,6 @@ static bool pci_check_and_mask_intx(struct
>> pci_dev *pdev)
>>  #define HAVE_PCI_IS_BRIDGE_API 1
>>  #endif
>>  
>> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>> -#define HAVE_ALLOC_IRQ_VECTORS 1
>> -#endif
>> -
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 3, 0)
>>  #define HAVE_MSI_LIST_IN_GENERIC_DEVICE 1
>>  #endif
>> @@ -136,3 +132,23 @@ static bool pci_check_and_mask_intx(struct
>> pci_dev *pdev)
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
>>  #define HAVE_PCI_MSI_MASK_IRQ 1
>>  #endif
>> +
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 8, 0)
>> +#define HAVE_ALLOC_IRQ_VECTORS 1
>> +#endif
>> +
>> +static inline bool igbuio_kernel_is_locked_down(void)
>> +{
>> +#ifdef CONFIG_LOCK_DOWN_KERNEL
>> +#ifdef CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT /* fedora */
>> +	return kernel_is_locked_down(NULL);
>> +#elif CONFIG_EFI_SECURE_BOOT_LOCK_DOWN /* ubuntu */
>> +	return kernel_is_locked_down();
>> +#else
>> +	return false;
>> +#endif
>> +#else
>> +	return false;
>> +#endif
>> +
>> +}
>> diff --git a/kernel/linux/igb_uio/igb_uio.c
>> b/kernel/linux/igb_uio/igb_uio.c
>> index cd9b7e721..b3233f18e 100644
>> --- a/kernel/linux/igb_uio/igb_uio.c
>> +++ b/kernel/linux/igb_uio/igb_uio.c
>> @@ -621,6 +621,11 @@ igbuio_pci_init_module(void)
>>  {
>>  	int ret;
>>  
>> +	if (igbuio_kernel_is_locked_down()) {
>> +		pr_err("Not able to use module, kernel lock down is
>> enabled\n");
>> +		return -EINVAL;
>> +	}
>> +
>>  	ret = igbuio_config_intr_mode(intr_mode);
>>  	if (ret < 0)
>>  		return ret;
> 
> kernel_is_locked_down already does print a message, so it seems a bit
> redundant (you can call it with something
> like kernel_is_locked_down("DPDK igb_uio kernel module")).

Yes, and no. There are two version of kernel_is_locked_down(), macro one gets
the string argument, the functions one doesn't get any paramter.

Two unify the message I think it is better to prevent kernel message by
providing NULL variable to macro and print log in igb_uio.


btw, I was thinking kernel_is_locked_down() difference is between fedora and
ubuntu but it is not, it seems related to the kernel version as well.

ubuntu-bionic (4.15.0) is using LOCK_DOWN_IN_EFI_SECURE_BOOT config option and
kernel_is_locked_down(char *msg) macro,
ubuntu-artful (4.13.16) is using EFI_SECURE_BOOT_LOCK_DOWN config option and
kernel_is_locked_down(void) function.

I will update the patch according.

> 
> In Debian Stretch the patches is the same as Ubuntu (Securelevel) but
> it didn't ship with any signed binaries so it's unlikely to be used.
> In Debian Buster the patchset is the same as Fedora (Lockdown).
>