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: Tue, 22 May 2018 16:23:50 +0100 Message-ID: References: <20180515165612.61243-1-ferruh.yigit@intel.com> <20180516114715.GA20004@hmswarspite.think-freely.org> <9a4178b1-f827-a9a9-ad03-96b8cc31f774@intel.com> <20180517073912.064c0a48@xeon-e3> <20180517194939.GC21980@hmswarspite.think-freely.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, Christian Ehrhardt , Luca Boccassi , Maxime Coquelin To: Neil Horman , Stephen Hemminger , Thomas Monjalon Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 171032BAF for ; Tue, 22 May 2018 17:23:54 +0200 (CEST) In-Reply-To: <20180517194939.GC21980@hmswarspite.think-freely.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/17/2018 8:49 PM, Neil Horman wrote: > On Thu, May 17, 2018 at 07:39:12AM -0700, Stephen Hemminger wrote: >> On Thu, 17 May 2018 14:23:46 +0100 >> Ferruh Yigit wrote: >> >>> 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. >> >> The issue is that igb_uio is not secure since it allows userspace to setup >> DMA to any physical address. In lockdown mode, even root is not supposed to be >> able to peek and poke arbitrary memory. >> >> Actually, it would make more sense to just have code to block all UIO drivers >> in uio.c since uio_pci_generic has the same issue. >> > That makes a bit more sense to me, yes. Hi Thomas, We can postpone this to next release, no need to get any risk related this for this release. Thanks, ferruh