linux-kernel - Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <10164.1480378260@warthog.procyon.org.uk>
Date:   Tue, 29 Nov 2016 00:11:00 +0000
From:   David Howells <dhowells@...hat.com>
To:     minyard@....org
Cc:     dhowells@...hat.com,
        One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
        keyrings@...r.kernel.org, matthew.garrett@...ula.com,
        linux-security-module@...r.kernel.org, linux-efi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed

Corey Minyard <minyard@....org> wrote:

> 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.

Okay, reasonable point.

> 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.

I could do that.  I presume you'd want hardcode_find_bmc() to return 1 in that
case without doing anything else.  Another possibility is to give a warning
and then clear ports[], addrs[] and irqs[].

> 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.

I don't think your suggested summary quite gets it right.  A lot of drivers,
sound drivers, for example, that aren't really critical can simply be
disabled - and some have to be disabled because there's no other way to
configure them.

It would have to be more like pr_err("Hard-coded device addresses, irqs and
dma channels are not permitted when the kernel is locked down."), possibly
with the addition of either "The driver has been disabled" or "These settings
have been ignored".

David