From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id CA48C7CEC for ; Thu, 17 May 2018 21:50:21 +0200 (CEST) Received: from cpe-2606-a000-111b-40b7-640c-26a-4e16-9225.dyn6.twc.com ([2606:a000:111b:40b7:640c:26a:4e16:9225] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1fJOui-0006Jz-7F; Thu, 17 May 2018 15:50:16 -0400 Date: Thu, 17 May 2018 15:49:39 -0400 From: Neil Horman To: Stephen Hemminger Cc: Ferruh Yigit , dev@dpdk.org, Christian Ehrhardt , Luca Boccassi , Maxime Coquelin Message-ID: <20180517194939.GC21980@hmswarspite.think-freely.org> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180517073912.064c0a48@xeon-e3> User-Agent: Mutt/1.9.5 (2018-04-13) X-Spam-Score: -2.9 (--) X-Spam-Status: No 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 19:50:23 -0000 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. Neil