DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Luca Boccassi <bluca@debian.org>, dev@dpdk.org
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH] igb_uio: fail and log if kernel lock down is enabled
Date: Wed, 16 May 2018 10:45:33 +0100	[thread overview]
Message-ID: <d92fbbf7-4707-3325-0914-f03e96afbae9@intel.com> (raw)
In-Reply-To: <1526406446.23337.115.camel@debian.org>

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).
> 

  reply	other threads:[~2018-05-16  9:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-15 16:56 Ferruh Yigit
2018-05-15 17:47 ` Luca Boccassi
2018-05-16  9:45   ` Ferruh Yigit [this message]
2018-05-16  9:56     ` Luca Boccassi
2018-05-15 18:52 ` Stephen Hemminger
2018-05-16  9:53   ` Ferruh Yigit
2018-05-16 10:18 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2018-05-16 10:50   ` Luca Boccassi
2018-05-16 14:42   ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
2018-05-17 11:34     ` Neil Horman
2018-05-17 13:26       ` Ferruh Yigit
2018-05-17 18:16         ` Neil Horman
2018-06-27 14:39     ` Thomas Monjalon
2018-06-29  7:04     ` David Marchand
2018-06-29  9:35       ` Ferruh Yigit
2018-05-16 11:47 ` [dpdk-dev] [PATCH] " Neil Horman
2018-05-17 13:23   ` Ferruh Yigit
2018-05-17 14:39     ` Stephen Hemminger
2018-05-17 19:49       ` Neil Horman
2018-05-22 15:23         ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d92fbbf7-4707-3325-0914-f03e96afbae9@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=bluca@debian.org \
    --cc=christian.ehrhardt@canonical.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).