Messages in this thread | | | Subject | Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed | From | Corey Minyard <> | Date | Mon, 28 Nov 2016 18:23:43 -0600 |
| |
On 11/28/2016 06:11 PM, David Howells wrote: > Corey Minyard <minyard@acm.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[]. >
Just returning -EPERM from that routine is fine, without doing anything else. You can basically just move your check to the top of that routine.
>> 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.
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.
-corey
| |