From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 13B426CD7 for ; Thu, 17 May 2018 15:23:49 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 May 2018 06:23:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,410,1520924400"; d="scan'208";a="41736336" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.77]) ([10.237.221.77]) by orsmga007.jf.intel.com with ESMTP; 17 May 2018 06:23:46 -0700 To: Neil Horman Cc: dev@dpdk.org, Christian Ehrhardt , Luca Boccassi , Maxime Coquelin , Stephen Hemminger References: <20180515165612.61243-1-ferruh.yigit@intel.com> <20180516114715.GA20004@hmswarspite.think-freely.org> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR3VQUJB33WBQAKCRD5M+tD3xNhH6DWEACVhEb8q1epPwZrUDoxzu7E TS1b8tmabOmnjXZRs6+EXgUVHkp2xxkCfDmL3pa5bC0G/74aJnWjNsdvE05V1cb4YK4kRQ62 FwDQ+hlrFrwFB3PtDZk1tpkzCRHvJgnIil+0MuEh32Y57ig6hy8yO8ql7Lohyrnpfk/nNpm4 jQGEF5qEeHcEFe1AZQlPHN/STno8NZSz2nl0b2cw+cujN1krmvB52Ah/2KugQ6pprVyrGrzB c34ZQO9OsmSjJlETCZk6EZzuhfe16iqBFbOSadi9sPcJRwaUQBid+xdFWl7GQ8qC3zNPibSF HmU43yBZUqJDZlhIcl6/cFpOSjv2sDWdtjEXTDn5y/0FsuY0mFE78ItC4kCTIVk17VZoywcd fmbbnwOSWzDq7hiUYuQGkIudJw5k/A1CMsyLkoUEGN3sLfsw6KASgS4XrrmPO4UVr3mH5bP1 yC7i1OVNpzvOxtahmzm481ID8sk72GC2RktTOHb0cX+qdoiMMfYgo3wRRDYCBt6YoGYUxF1p msjocXyqToKhhnFbXLaZlVfnQ9i2i8jsj9SKig+ewC2p3lkPj6ncye9q95bzhmUeJO6sFhJg Hiz6syOMg8yCcq60j07airybAuHIDNFWk0gaWAmtHZxLObZx2PVn2nv9kLYGohFekw0AOsIW ta++5m48dnCoAc7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKMLBZQQYAQIADwIbDAUCWZR3hQUJA59eRwAKCRD5M+tD3xNhH4a/D/4jLAZu UhvU1swWcNEVVCELZ0D3LOV14XcY2MXa3QOpeZ9Bgq7YYJ4S5YXK+SBQS0FkRZdjGNvlGZoG ZdpU+NsQmQFhqHGwX0IT9MeTFM8uvKgxNKGwMVcV9g0IOqwBhGHne+BFboRA9362fgGW5AYQ zT0mzzRKEoOh4r3AQvbM6kLISxo0k1ujdYiI5nj/5WoKDqxTwwfuN1uDUHsWo3tzenRmpMyU NyW3Dc+1ajvXLyo09sRRq7BnM99Rix1EGL8Qhwy+j0YAv+FuspWxUX9FxXYho5PvGLHLsHfK FYQ7x/RRbpMjkJWVfIe/xVnfvn4kz+MTA5yhvsuNi678fLwY9hBP0y4lO8Ob2IhEPdfnTuIs tFVxXuelJ9xAe5TyqP0f+fQjf1ixsBZkqOohsBXDfje0iaUpYa/OQ/BBeej0dUdg2JEu4jAC x41HpVCnP9ipLpD0fYz1d/dX0F/VY2ovW6Eba/y/ngOSAR6C+u881m7oH2l0G47MTwkaQCBA bLGXPj4TCdX3lftqt4bcBPBJ+rFAnJmRHtUuyyaewBnZ81ZU2YAptqFM1kTh+aSvMvGhfVsQ qZL2rk2OPN1hg+KXhErlbTZ6oPtLCFhSHQmuxQ4oc4U147wBTUuOdwNjtnNatUhRCp8POc+3 XphVR5G70mnca1E2vzC77z+XSlTyRA== Message-ID: <9a4178b1-f827-a9a9-ad03-96b8cc31f774@intel.com> Date: Thu, 17 May 2018 14:23:46 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180516114715.GA20004@hmswarspite.think-freely.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Thu, 17 May 2018 13:23:50 -0000 On 5/16/2018 12:47 PM, Neil Horman wrote: > On Tue, May 15, 2018 at 05:56:12PM +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 >> > What exactly is the error you get when you load the igb_uio module? I ask > because, looking at least at the Fedora patches, the BAR registers themselves > aren't made unwriteable, its only userspace access through very specific > channels that are gated on (things like /proc/bus/pci/...). From what I can see > (again, not having looked at other implementations), kernel modules that load > successfully should be able to modify bar registers, and otherwise function > normally (as to weather they are permitted to load is another question). This patch is based on understanding on the effect of the lockdown patches, that it will disable hardware access from userspace. I don't have an environment to test this and indeed I am not very clear about effects of the lockdown set. > > The reason I ask this is twofold: > > 1) if a specific access is failing, that seems like it could be the trigger to > use, rather than explicitly checking if the kernel is locked down. I don't see > one expressly called, but if you're calling pci_write_config_* somewhere, and > getting an EPERM error, thats a reason to fail the loading of igb_uio, based on > the fact that you don't have permission to write to the appropriate hardware. > > 2) Its more than just the igb_uio module that will fail. Any attempt to pass a > VF into a guest using user space tools (including the vfio scripts that dpdk > includes), should fail. As such, it might be better to have some component in > user space test one of the aforementioned restricted paths for writeability. > Such an approach would be more generic, and eliminate the need to assemble a set > of tests to see if the kernel is locked down. A more generic error message > could then be logged and the dpdk could exit gracefully, weather or not igb_uio > was loaded. With the existing patches, expectation is vfio will work but it will only effect igb_uio. > > Its probably also important to note here that, this lockdown patch, from my > digging, has been carried in Fedora since December of 2016, and its still not > made it upstream. Thats not to say that it will never do so, but it suggests > that, given the 2 years of out of tree updates its received, there its use is > both very specific and limted to users who understand its implications. This > probably isn't something to make significant or hard-to-maintain changes to the > dpdk (or any other software) over. Have same expectation that use will be specific and limited, that is why planed to change only igb_uio to detect the case and return with a log, instead of updating anything in the dpdk. in igb_uio the plan was just adding simple check, patches being not upstreamed added more complexity, but not still I believe it is not significant or hard-to-maintain change. > > Neil > >> 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; >> -- >> 2.14.3 >> >>