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

From: Corey Minyard
Date: Mon Nov 28 2016 - 19:23:59 EST

On 11/28/2016 06:11 PM, David Howells wrote:
Corey Minyard <minyard@xxxxxxx> 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
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[].

Just returning -EPERM from that routine is fine, without doing anything
else. You can basically just move your check to the top of that

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

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.

Yeah. My main issue was that the sysadmin would see this and not
have any idea what was going on.

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

That sounds better than what I had.