From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932214AbcK1Wcd (ORCPT ); Mon, 28 Nov 2016 17:32:33 -0500 Received: from mail-oi0-f68.google.com ([209.85.218.68]:32795 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752827AbcK1WcX (ORCPT ); Mon, 28 Nov 2016 17:32:23 -0500 Reply-To: minyard@acm.org Subject: Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed References: <20161116222731.563fb85e@lxorguk.ukuu.org.uk> <147933283664.19316.12454053022687659937.stgit@warthog.procyon.org.uk> <26173.1479769852@warthog.procyon.org.uk> To: David Howells , One Thousand Gnomes Cc: keyrings@vger.kernel.org, matthew.garrett@nebula.com, linux-security-module@vger.kernel.org, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org From: Corey Minyard Message-ID: Date: Mon, 28 Nov 2016 16:32:19 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <26173.1479769852@warthog.procyon.org.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/21/2016 05:10 PM, David Howells wrote: > One Thousand Gnomes wrote: > >> You need to filter or lock down kernel module options because a lot of >> modules let you set the I/O port or similar (eg mmio) which means you can >> hack the entire machine with say the 8250 driver just by using it with an >> mmio of the right location to patch the secure state to zero just by >> getting the ability to write to the modules conf file. > Is the attached patch the right sort of idea? [Note that I haven't actually > compiled most of these drivers to check my changes yet.] > > David > --- snip > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c > index a112c0146012..7fb9c299a183 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -3725,6 +3725,12 @@ static int init_ipmi_si(void) > struct smi_info *e; > enum ipmi_addr_src type = SI_INVALID; > > + if ((num_addrs || num_ports || num_irqs) && > + kernel_is_locked_down()) { > + pr_err(PFX "Kernel is locked down\n"); > + return -EPERM; > + } > + > if (initialized) > return 0; > initialized = 1; This would prevent any IPMI interface from working if any address was given on the kernel command line. I'm not sure what the best policy is, but that sounds like a possible DOS to me. Can you put this check in hardcode_find_bmc()? Thats the only place where the hardcoded addresses are used, and a check there won't affect anything else. Also, the error message sounds a little vague to me. If I was a sysadmin and got this, I wouldn't be sure what was going on. Maybe something like: The kernel is locked down, but hard-coded device addresses were given on the driver command line. Ignoring these, but this is a possible security issue. That's fairly wordy, but it gets the point across. You could also move the pr_err() into kernel_is_locked_down() and pass in the prefix, since there is basically the same pr_err() after every check. Thanks, -corey