From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] igb_uio: fail and log if kernel lock down is enabled Date: Wed, 16 May 2018 10:45:33 +0100 Message-ID: References: <20180515165612.61243-1-ferruh.yigit@intel.com> <1526406446.23337.115.camel@debian.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Christian Ehrhardt , Maxime Coquelin , Neil Horman , Stephen Hemminger To: Luca Boccassi , dev@dpdk.org Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 37A491B1D5 for ; Wed, 16 May 2018 11:45:37 +0200 (CEST) In-Reply-To: <1526406446.23337.115.camel@debian.org> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >> --- >> Cc: Christian Ehrhardt >> Cc: Luca Boccassi >> Cc: Maxime Coquelin >> Cc: Neil Horman >> Cc: Stephen Hemminger >> --- >>  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). >