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 <ferruh.yi...@intel.com> 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
