From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f196.google.com (mail-wr0-f196.google.com [209.85.128.196]) by dpdk.org (Postfix) with ESMTP id 751641B3D9 for ; Wed, 16 May 2018 11:56:04 +0200 (CEST) Received: by mail-wr0-f196.google.com with SMTP id v15-v6so208394wrm.10 for ; Wed, 16 May 2018 02:56:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:mime-version; bh=IIvq1EO17jl2i0tkvlmkD7UoEk9KFiB15rTk3ng2OEc=; b=tx/dWJ9Uten+5t+tHXn7xwOYFlWuJ/DlkFmthZvp5NWfdtLlDjUKQodvDNyBrph92L Mn1ePd+fdRcaAbj0r/IIkZXVMLGRgRE27jEdRv+KO9ATWeuPslDHRy9ARXkxZhw6Lhav sj3vu5xmDQqA48Jq17ukR9hbCGIvSAcMxI+PiFQ1OsiYdgCQ5BoHdvwYhjgYvCyoTGjf QwOBp41dYx5Gua7OQOfJ74S/PEDZP0OYir171k1OeItJ1RV3xdOMujVDQjMo4Pt0vnvx y8sxpDMVIaj9oxHboYLXWfB3a5Mb6IBY9v26RjIRYhBFqLtjbwWr8e2ZqvWtkGsdLczt cY5w== X-Gm-Message-State: ALKqPweS2Z8LgLprzfRBk/f+QoC3/s8pd09O8cY8qLExBXeCJ5uri8xY Vs44TePAb9uB8udkF5F7hFw= X-Google-Smtp-Source: AB8JxZqFy4pCIOfBxmAiaf8uo3M+V+RSKQYaJPOPlU5QxT/ZkkXHh6R5Lin62/Bn5wKL3vZJZG3LaA== X-Received: by 2002:adf:c412:: with SMTP id v18-v6mr225023wrf.20.1526464564097; Wed, 16 May 2018 02:56:04 -0700 (PDT) Received: from localhost (slip139-92-244-193.lon.uk.prserv.net. [139.92.244.193]) by smtp.gmail.com with ESMTPSA id v18-v6sm3370077wrf.76.2018.05.16.02.56.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 16 May 2018 02:56:02 -0700 (PDT) Message-ID: <1526464560.23337.121.camel@debian.org> From: Luca Boccassi To: Ferruh Yigit , dev@dpdk.org Cc: Christian Ehrhardt , Maxime Coquelin , Neil Horman , Stephen Hemminger Date: Wed, 16 May 2018 10:56:00 +0100 In-Reply-To: References: <20180515165612.61243-1-ferruh.yigit@intel.com> <1526406446.23337.115.camel@debian.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.22.6-1+deb9u1 Mime-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] igb_uio: fail and log if kernel lock down is enabled X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 May 2018 09:56:04 -0000 On Wed, 2018-05-16 at 10:45 +0100, Ferruh Yigit wrote: > 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. > > >=20 > > > Lock down patches are not part of the vanilla kernel but they are > > > applied and used by some distros already [1]. > > >=20 > > > 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. > > >=20 > > > 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. > > >=20 > > > [1] > > > kernel.ubuntu.com/git/ubuntu/ubuntu- > > > artful.git/commit/?id=3D99f9ef18d5b6 > > > And a few more patches to > > >=20 > > > Signed-off-by: Ferruh Yigit > > > --- > > > Cc: Christian Ehrhardt > > > Cc: Luca Boccassi > > > Cc: Maxime Coquelin > > > Cc: Neil Horman > > > Cc: Stephen Hemminger > > > --- > > > =C2=A0kernel/linux/igb_uio/compat.h=C2=A0=C2=A0| 24 +++++++++++++++++= +++---- > > > =C2=A0kernel/linux/igb_uio/igb_uio.c |=C2=A0=C2=A05 +++++ > > > =C2=A02 files changed, 25 insertions(+), 4 deletions(-) > > >=20 > > > 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) > > > =C2=A0#define HAVE_PCI_IS_BRIDGE_API 1 > > > =C2=A0#endif > > > =C2=A0 > > > -#if LINUX_VERSION_CODE >=3D KERNEL_VERSION(4, 8, 0) > > > -#define HAVE_ALLOC_IRQ_VECTORS 1 > > > -#endif > > > - > > > =C2=A0#if LINUX_VERSION_CODE >=3D KERNEL_VERSION(4, 3, 0) > > > =C2=A0#define HAVE_MSI_LIST_IN_GENERIC_DEVICE 1 > > > =C2=A0#endif > > > @@ -136,3 +132,23 @@ static bool pci_check_and_mask_intx(struct > > > pci_dev *pdev) > > > =C2=A0#if LINUX_VERSION_CODE >=3D KERNEL_VERSION(4, 5, 0) > > > =C2=A0#define HAVE_PCI_MSI_MASK_IRQ 1 > > > =C2=A0#endif > > > + > > > +#if LINUX_VERSION_CODE >=3D 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) > > > =C2=A0{ > > > =C2=A0 int ret; > > > =C2=A0 > > > + if (igbuio_kernel_is_locked_down()) { > > > + pr_err("Not able to use module, kernel lock down > > > is > > > enabled\n"); > > > + return -EINVAL; > > > + } > > > + > > > =C2=A0 ret =3D igbuio_config_intr_mode(intr_mode); > > > =C2=A0 if (ret < 0) > > > =C2=A0 return ret; > >=20 > > kernel_is_locked_down already does print a message, so it seems a > > bit > > redundant (you can call it with something > > like=C2=A0kernel_is_locked_down("DPDK igb_uio kernel module")). >=20 > 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. >=20 > 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. >=20 >=20 > 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. >=20 > 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. >=20 > I will update the patch according. Yes, they all use the same patchset, which originally was called "securelevel" and was the first version used initially when distros started to ship with secure boot support. In the past couple of years it's been reworked and it's now called "lockdown". Expect all distros to move to the newer version at some point in time. --=20 Kind regards, Luca Boccassi