| Message ID | 148059538747.31612.8974972913601108271.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 01, 2016 at 12:29:47PM +0000, David Howells wrote: > Provided an annotation for module parameters that specify hardware > parameters (such as io ports, iomem addresses, irqs, dma channels, fixed > dma buffers and other types). > > This will enable such parameters to be locked down in the core parameter > parser for secure boot support. ick ick ick. First off, this "secure boot support" massive patchset has not gone anywhere yet, so why do this now? Also, I think Alan's comment about it the last time it came up was more like a "look at all of the other ways you could do bad things to hardware!" comment, not a "you need to also do this thing too!" type of request. I certianly do not see how this makes anything "more secure" at all. And I thought the last time this came up, Linus also objected to it, which is why the patchset never went anywhere. Secure boot is a trust that the previous boot process is now booting your image that it feels is secure (with various levels of "secure"). It is not about "lock things down so no one can ever touch the hardware through different options, except through random logic[1] that we somehow trust "more" than configuration options. So, what are you really trying to "block" here? The ability for someone to set an i/o port value? why? Why does it matter what root sets for an irq? For a dma buffer? For anything else? What is preventing this going to "secure" somehow? Overall, I really don't like this, and honestly, don't like the whole "secure boot" patchset either, as it is really a lot of work for absolutely no gain that I can see. Who is "asking" for this type of thing, and what are their specific requirements? thanks, greg k-h [1] Really, do you trust random driver writers to get things more "correct" than allowing people to get their hardware to work properly with module parameters? I know driver writers, and really, I trust users more than them... -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg KH <gregkh@linuxfoundation.org> wrote: > Also, I think Alan's comment about it the last time it came up was more like > a "look at all of the other ways you could do bad things to hardware!" > comment, not a "you need to also do this thing too!" type of request. Alan said: 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. I'm not entirely sure how one would do it, but Alan seems to think it can be done. > First off, this "secure boot support" massive patchset has not gone > anywhere yet, so why do this now? To continue quoting Alan: Without that at least fixed I don't see the point in merging this. Either we don't do it (which given the level of security the current Linux kernel provides, and also all the golden key messups from elsewhere might be the honest approach), or at least try and do the job right. Alan also said this: It is - so pushing something with known trivial holes isn't a useful way to do this. The module parameter hole needs to be addressed before this is fit for upstream. So you and Alan present something of a conflict of ordering: for Alan, I have to fix the module parameter hole first; for you, I have to do the secure boot support first. > So, what are you really trying to "block" here? The ability for someone > to set an i/o port value? why? Why does it matter what root sets for > an irq? For a dma buffer? For anything else? What is preventing this > going to "secure" somehow? I'll grant that prohibiting the changing of irq settings or dma channel settings may not actually be necessary. However, annotation module parameters to indicate hardware resource configuration seems potentially useful in its own right - and lets the policy be decided later. Setting ioports and iomem might allow you to get a driver for one piece of hardware to do something nasty with an unrelated piece of hardware. I really need Alan to weigh in on this. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 01, 2016 at 04:01:35PM +0100, Greg KH wrote: > First off, this "secure boot support" massive patchset has not gone > anywhere yet, so why do this now? Because David ended up with the short straw when distro maintainers talked about this at LPC. > Secure boot is a trust that the previous boot process is now booting > your image that it feels is secure (with various levels of "secure"). > It is not about "lock things down so no one can ever touch the hardware > through different options, except through random logic[1] that we > somehow trust "more" than configuration options. If root is able to modify the behaviour of verified code after it was verified, then the value of that verification is reduced. Ensuring that the code remains trustworthy is vital in a number of security use cases. > So, what are you really trying to "block" here? The ability for someone > to set an i/o port value? why? Why does it matter what root sets for > an irq? For a dma buffer? For anything else? What is preventing this > going to "secure" somehow? If root can tell a driver to probe for hardware at a specific address, and that driver will then blindly do so, root is trivially able to modify arbitrary kernel memory and disable arbitrary security features. IRQ or io port attacks are much more difficult to take advantage of, but I could imagine that some of them are still plausible. > Overall, I really don't like this, and honestly, don't like the whole > "secure boot" patchset either, as it is really a lot of work for > absolutely no gain that I can see. Who is "asking" for this type of > thing, and what are their specific requirements? Here's an example. The sysfs option to enable module signing is write once. If root sets that, root can't unset it. Except there's a whole bunch of ways that root *can* unset it, including kexec (https://mjg59.dreamwidth.org/28746.html) and a bunch of other things that are disabled by this patchset. That feature is entirely useless as is. This patchset helps make it useful. Right now, the secure boot patchset is shipped by basically every single mainstream Linux distribution (and a whole bunch that are niche). Right now they're having to do extra work to rebase it and ensure that fixes get distributed to everyone. There's clearly demand, and Linus has been clear that features that are shipped by everyone should just go into mainline, so if there are *technical* objections then let's figure them out and otherwise just get this stuff merged. -- Matthew Garrett | mjg59@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2016 at 03:07:00AM +0000, Matthew Garrett wrote: > On Thu, Dec 01, 2016 at 04:01:35PM +0100, Greg KH wrote: > > > First off, this "secure boot support" massive patchset has not gone > > anywhere yet, so why do this now? > > Because David ended up with the short straw when distro maintainers > talked about this at LPC. > > > Secure boot is a trust that the previous boot process is now booting > > your image that it feels is secure (with various levels of "secure"). > > It is not about "lock things down so no one can ever touch the hardware > > through different options, except through random logic[1] that we > > somehow trust "more" than configuration options. > > If root is able to modify the behaviour of verified code after it was > verified, then the value of that verification is reduced. Ensuring that > the code remains trustworthy is vital in a number of security use cases. Ok, but why are you now deciding to somehow try to "classify" the types of module parameters? Why would you want to allow irqs to change, but not iobase? Or something else? Who is going to do this "I want you and you but not you" decision? Why not just forbid all module parameters at all if they are so dangerous? > > So, what are you really trying to "block" here? The ability for someone > > to set an i/o port value? why? Why does it matter what root sets for > > an irq? For a dma buffer? For anything else? What is preventing this > > going to "secure" somehow? > > If root can tell a driver to probe for hardware at a specific address, > and that driver will then blindly do so, root is trivially able to > modify arbitrary kernel memory and disable arbitrary security features. > IRQ or io port attacks are much more difficult to take advantage of, but > I could imagine that some of them are still plausible. Then just mark them all as "bad", why pick and choose? > > Overall, I really don't like this, and honestly, don't like the whole > > "secure boot" patchset either, as it is really a lot of work for > > absolutely no gain that I can see. Who is "asking" for this type of > > thing, and what are their specific requirements? > > Here's an example. The sysfs option to enable module signing is write > once. If root sets that, root can't unset it. Except there's a whole > bunch of ways that root *can* unset it, including kexec > (https://mjg59.dreamwidth.org/28746.html) and a bunch of other things > that are disabled by this patchset. That feature is entirely useless as > is. This patchset helps make it useful. "this" patchset does nothing to disable anything, so I can't speak to any of the other goals you might have for that code, that's not what we are reviewing here. > Right now, the secure boot patchset is shipped by basically every single > mainstream Linux distribution (and a whole bunch that are niche). Right > now they're having to do extra work to rebase it and ensure that fixes > get distributed to everyone. There's clearly demand, and Linus has been > clear that features that are shipped by everyone should just go into > mainline, so if there are *technical* objections then let's figure them > out and otherwise just get this stuff merged. "this stuff" is brand new things, that no one is shipping. And nothing "just goes" into mainline, no matter what foolish stuff distros end up shipping (an example, do you want the giant Xen kernel patchset that SuSE has been dragging around for 10+ years?) Come on, you know better than this, each patch/series/feature has to be justifable on it's own, and this patchset, as-is, doesn't pass that test to me, if for no other reason than it is just "marking" things that is never then being used. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2016 at 07:55:30AM +0100, Greg KH wrote: > On Fri, Dec 02, 2016 at 03:07:00AM +0000, Matthew Garrett wrote: > > If root is able to modify the behaviour of verified code after it was > > verified, then the value of that verification is reduced. Ensuring that > > the code remains trustworthy is vital in a number of security use cases. > > Ok, but why are you now deciding to somehow try to "classify" the types > of module parameters? Why would you want to allow irqs to change, but > not iobase? Or something else? Who is going to do this "I want you and > you but not you" decision? Why not just forbid all module parameters at > all if they are so dangerous? If the parameters plausibly make it possible for root to modify the kernel in interesting ways, then restricting them makes sense. My gut sense is that parameters that allow the alteration of the base address of memory mapped devices are clearly a problem in this respect, but port io and IRQs *probably* aren't. On the other hand, blocking mmio base addresses and not blocking the others is kind of inconsistent. > > If root can tell a driver to probe for hardware at a specific address, > > and that driver will then blindly do so, root is trivially able to > > modify arbitrary kernel memory and disable arbitrary security features. > > IRQ or io port attacks are much more difficult to take advantage of, but > > I could imagine that some of them are still plausible. > > Then just mark them all as "bad", why pick and choose? Most parameters are going to be fine, but sure, flagging all IRQ/mmio/pio address parameters seems reasonable. > > Here's an example. The sysfs option to enable module signing is write > > once. If root sets that, root can't unset it. Except there's a whole > > bunch of ways that root *can* unset it, including kexec > > (https://mjg59.dreamwidth.org/28746.html) and a bunch of other things > > that are disabled by this patchset. That feature is entirely useless as > > is. This patchset helps make it useful. > > "this" patchset does nothing to disable anything, so I can't speak to > any of the other goals you might have for that code, that's not what we > are reviewing here. This is prep work that makes it possible to block module parameters that would otherwise make it possible to avoid those restrictions. > > Right now, the secure boot patchset is shipped by basically every single > > mainstream Linux distribution (and a whole bunch that are niche). Right > > now they're having to do extra work to rebase it and ensure that fixes > > get distributed to everyone. There's clearly demand, and Linus has been > > clear that features that are shipped by everyone should just go into > > mainline, so if there are *technical* objections then let's figure them > > out and otherwise just get this stuff merged. > > "this stuff" is brand new things, that no one is shipping. And nothing > "just goes" into mainline, no matter what foolish stuff distros end up > shipping (an example, do you want the giant Xen kernel patchset that > SuSE has been dragging around for 10+ years?) This is a logical extension to the base patchset, and one maintainer has NAKed the base patchset due to it lacking this feature. If you don't care about this then just tell Alan that you want the base patchset merged anyway and we'll go from there. Let's not get into a situation where people are being given incompatible requirements before something's merged.
Greg KH <gregkh@linuxfoundation.org> wrote: > > If root is able to modify the behaviour of verified code after it was > > verified, then the value of that verification is reduced. Ensuring that > > the code remains trustworthy is vital in a number of security use cases. > > Ok, but why are you now deciding to somehow try to "classify" the types > of module parameters? Because Alan says that locking down the module parameters needs to be done first. Since I had to go through and modify each module parameter to mark the hardware config ones, it seemed like a good opportunity to label their type (ioport, iomem, irq, etc.) whilst I was at it. > Then just mark them all as "bad", why pick and choose? Because some drivers, IPMI for example, can also be autoconfigured via PCI, PNP, ACPI or whatever and still be useful, if not important, to the system's operation. Simply marking all drivers that can be so configured as "bad" and rejecting them outright in lockdown mode is a non-starter. > "this" patchset does nothing to disable anything, That is correct, I even say as much in the cover note and patch 1. > so I can't speak to any of the other goals you might have for that code, > that's not what we are reviewing here. With this patchset, I'm hoping maintainers will check the annotations are correct and point out anything I've missed. There are a lot of module parameters and not so much consistency in schemes for naming parameters and their variables. > > Right now, the secure boot patchset > > "this stuff" is brand new things, that no one is shipping. True, but my other two patchsets are primarily made up of things people *are* shipping. If you're happy to for those to go in and can persuade Alan to okay deferral of module parameter lockdown for an extra cycle, that's fine by me. > Come on, you know better than this, each patch/series/feature has to be > justifable on it's own, and this patchset, as-is, doesn't pass that test > to me, if for no other reason than it is just "marking" things that is > never then being used. You're being unreasonable. The complete set is on the order of 90 patches, I think. I could submit them all in one go in a single series, but then people would be complaining that it's too big and that I have to split it up. I have broken it up into a number of logical series, of which I've published some: (1) Determining the EFI secure boot state. This only depends on tip efi/core. This mostly takes what the ARM arch already does upstream and extends it to x86 too. (2) Marking hardware config module params. Patches 2+ all depend on patch 1, but there are no dependencies outside of that series. If I could get patch 1 upstream, I could distribute patches 2+ individually to the maintainers. (3) Kernel lockdown. This takes the determination made by (1) and applies it, enabling various lockdowns, including locking down anything annotated in (2). (4) System blacklist. List hashes to be blacklisted. This is independent of all other series. (5) UEFI/SHIM whitelist/blacklist loading. This has a dependency on some constants added in (1). I have to do them in some order. Doing it this way means that some of these are self-contained, making it technically easier to upstream those pieces. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2016 at 02:59:22PM +0000, David Howells wrote: > Greg KH <gregkh@linuxfoundation.org> wrote: > > > > If root is able to modify the behaviour of verified code after it was > > > verified, then the value of that verification is reduced. Ensuring that > > > the code remains trustworthy is vital in a number of security use cases. > > > > Ok, but why are you now deciding to somehow try to "classify" the types > > of module parameters? > > Because Alan says that locking down the module parameters needs to be done > first. Since I had to go through and modify each module parameter to mark the > hardware config ones, it seemed like a good opportunity to label their type > (ioport, iomem, irq, etc.) whilst I was at it. > > > Then just mark them all as "bad", why pick and choose? > > Because some drivers, IPMI for example, can also be autoconfigured via PCI, > PNP, ACPI or whatever and still be useful, if not important, to the system's > operation. > > Simply marking all drivers that can be so configured as "bad" and rejecting > them outright in lockdown mode is a non-starter. Sorry, I meant to mark all of these types of attributes as "bad", not trying to classify all of the different types of attributes, given that you will probably want to just ban all of them or none, right? > > > Right now, the secure boot patchset > > > > "this stuff" is brand new things, that no one is shipping. > > True, but my other two patchsets are primarily made up of things people *are* > shipping. If you're happy to for those to go in and can persuade Alan to okay > deferral of module parameter lockdown for an extra cycle, that's fine by me. I'll defer to Alan as to what he feels is needed here, given that this patchset isn't being shipped by anyone I think it's odd to somehow make this a pre-requisite for anything to be merged as no real user of the patchset seemed to feel this type of thing was needed :) > > Come on, you know better than this, each patch/series/feature has to be > > justifable on it's own, and this patchset, as-is, doesn't pass that test > > to me, if for no other reason than it is just "marking" things that is > > never then being used. > > You're being unreasonable. The complete set is on the order of 90 patches, I > think. I could submit them all in one go in a single series, but then people > would be complaining that it's too big and that I have to split it up. > > I have broken it up into a number of logical series, of which I've published > some: > > (1) Determining the EFI secure boot state. This only depends on tip > efi/core. This mostly takes what the ARM arch already does upstream and > extends it to x86 too. > > (2) Marking hardware config module params. Patches 2+ all depend on patch 1, > but there are no dependencies outside of that series. If I could get > patch 1 upstream, I could distribute patches 2+ individually to the > maintainers. > > (3) Kernel lockdown. This takes the determination made by (1) and applies > it, enabling various lockdowns, including locking down anything annotated > in (2). > > (4) System blacklist. List hashes to be blacklisted. This is independent of > all other series. These are hashes of what? > (5) UEFI/SHIM whitelist/blacklist loading. This has a dependency on some > constants added in (1). > > I have to do them in some order. Doing it this way means that some of these > are self-contained, making it technically easier to upstream those pieces. I think patch 3 is going to be the "hardest", along with 2, given the large area it touches. Why not work on the other bits first? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 01 Dec 2016 16:02:26 +0000 David Howells <dhowells@redhat.com> wrote: > Greg KH <gregkh@linuxfoundation.org> wrote: > > > Also, I think Alan's comment about it the last time it came up was more like > > a "look at all of the other ways you could do bad things to hardware!" > > comment, not a "you need to also do this thing too!" type of request. In all honesty I think both need to go in together, otherwise the first patch is useless. It's not a case of "oh there may be another obscure exploit .." , this is "I can automate it with a python script, post a CVE, and show I'm awesome" 8) Alan -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> If the parameters plausibly make it possible for root to modify the > kernel in interesting ways, then restricting them makes sense. My gut > sense is that parameters that allow the alteration of the base address > of memory mapped devices are clearly a problem in this respect, but port > io and IRQs *probably* aren't. On the other hand, blocking mmio base > addresses and not blocking the others is kind of inconsistent. It is actually useful even without secure boot because right now DAC capability bypass is easier to get and gives you CAP_SYS_RAWIO if you've mastered your first class in being an 3733t h4x0r. (because you can modify /etc/moprobe.d/* and even with signed modules that's not protected). It's also the case if you go through those drivers that pretty much none of them are found on a modern EFI and secure boot enabled system and those that are will be using ACPI, PCI, devicetree or other reliablish bus enumerations instead. The cross section of people needing io=foo, and having secure boot is close to nil. > This is a logical extension to the base patchset, and one maintainer has > NAKed the base patchset due to it lacking this feature. If you don't > care about this then just tell Alan that you want the base patchset > merged anyway and we'll go from there. Let's not get into a situation > where people are being given incompatible requirements before > something's merged. The base patchset actually doesn't do anything without it. I'd hope the vendors adopt it anyway if they are serious about secure boot (or security in general) given it's really valuable even without the secure boot firmware nonsense to be able to boot a machine and then lock down raw I/O access. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2016 at 09:12:27PM +0000, One Thousand Gnomes wrote: > On Thu, 01 Dec 2016 16:02:26 +0000 > David Howells <dhowells@redhat.com> wrote: > > > Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > Also, I think Alan's comment about it the last time it came up was more like > > > a "look at all of the other ways you could do bad things to hardware!" > > > comment, not a "you need to also do this thing too!" type of request. > > > In all honesty I think both need to go in together, otherwise the first > patch is useless. It's not a case of "oh there may be another obscure > exploit .." , this is "I can automate it with a python script, post a > CVE, and show I'm awesome" 8) What about all of the ways you can change ioports dynamically from ioctls? Or can't python write ioctls to device nodes? :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg KH <gregkh@linuxfoundation.org> wrote: > What about all of the ways you can change ioports dynamically from > ioctls? Or can't python write ioctls to device nodes? :) Do you mean change the ioport a driver uses by ioctl or actually read/write an ioport directly? Do the following patches that I've already posted address your issues: http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=c67c338dd82d28c67d38eb3147368eb36dbf1c16 http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=10bd7277eef5194ba038fc2d907bac9e6aeab12b They're going to be in a patchset that I am/was intending to sit atop the module parameter-lockdown patchset. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2016 at 10:42:47AM +0000, David Howells wrote: > Greg KH <gregkh@linuxfoundation.org> wrote: > > > What about all of the ways you can change ioports dynamically from > > ioctls? Or can't python write ioctls to device nodes? :) > > Do you mean change the ioport a driver uses by ioctl or actually read/write an > ioport directly? change the ioport a driver uses. The tty layer can do this for UARTs through an ioctl (can't remember which one off the top of my head, sorry, it gets reported as a bug by the syscall fuzzers every other year or so when they crash the kernel randomly...) > Do the following patches that I've already posted address your issues: > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=c67c338dd82d28c67d38eb3147368eb36dbf1c16 > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=10bd7277eef5194ba038fc2d907bac9e6aeab12b > > They're going to be in a patchset that I am/was intending to sit atop the > module parameter-lockdown patchset. Ah, I hadn't seen those, that's a good start, and does close some other places. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg KH <gregkh@linuxfoundation.org> wrote: > > Because Alan says that locking down the module parameters needs to be done > > first. Since I had to go through and modify each module parameter to mark the > > hardware config ones, it seemed like a good opportunity to label their type > > (ioport, iomem, irq, etc.) whilst I was at it. > > > > > Then just mark them all as "bad", why pick and choose? > > > > Because some drivers, IPMI for example, can also be autoconfigured via PCI, > > PNP, ACPI or whatever and still be useful, if not important, to the system's > > operation. > > > > Simply marking all drivers that can be so configured as "bad" and rejecting > > them outright in lockdown mode is a non-starter. > > Sorry, I meant to mark all of these types of attributes as "bad", not > trying to classify all of the different types of attributes, given that > you will probably want to just ban all of them or none, right? That was my intent. However, since I was touching every hardware module param anyway, it seemed like a good thing to add. At least now one can grep for every config param that explicitly sets a DMA channel, for example. > I'll defer to Alan as to what he feels is needed here, given that this > patchset isn't being shipped by anyone I think it's odd to somehow make > this a pre-requisite for anything to be merged as no real user of the > patchset seemed to feel this type of thing was needed :) But unless I post it, no one's going to review it. Granted, I should probably have stuck "RFC" in the subject. > > (4) System blacklist. List hashes to be blacklisted. This is independent > > of all other series. > > These are hashes of what? Hashes of module content, kexec image content, X.509 toBeSigned content, firmware blobs. Things that are going to get hashes blacklisted in the UEFI database. > I think patch 3 is going to be the "hardest", along with 2, given the > large area it touches. Why not work on the other bits first? Because not everyone agrees with you on the order. Further, some of the bits are independent - (2) vs (4)/(5) for example. Besides, I have patchsets for (1) and (3). There is a patch in (3) that depends on (2), but that could be moved across. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 52666d90ca94..6be1949ebcdf 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -60,9 +60,11 @@ struct kernel_param_ops { * Flags available for kernel_param * * UNSAFE - the parameter is dangerous and setting it will taint the kernel + * HWPARAM - Hardware param not permitted in lockdown mode */ enum { - KERNEL_PARAM_FL_UNSAFE = (1 << 0) + KERNEL_PARAM_FL_UNSAFE = (1 << 0), + KERNEL_PARAM_FL_HWPARAM = (1 << 1), }; struct kernel_param { @@ -451,6 +453,67 @@ extern int param_set_bint(const char *val, const struct kernel_param *kp); perm, -1, 0); \ __MODULE_PARM_TYPE(name, "array of " #type) +enum hwparam_type { + hwparam_ioport, /* Module parameter configures an I/O port */ + hwparam_iomem, /* Module parameter configures an I/O mem address */ + hwparam_ioport_or_iomem, /* Module parameter could be either, depending on other option */ + hwparam_irq, /* Module parameter configures an I/O port */ + hwparam_dma, /* Module parameter configures a DMA channel */ + hwparam_dma_addr, /* Module parameter configures a DMA buffer address */ + hwparam_other, /* Module parameter configures some other value */ +}; + +/** + * module_param_hw_named - A parameter representing a hw parameters + * @name: a valid C identifier which is the parameter name. + * @value: the actual lvalue to alter. + * @type: the type of the parameter + * @hwtype: what the value represents (enum hwparam_type) + * @perm: visibility in sysfs. + * + * Usually it's a good idea to have variable names and user-exposed names the + * same, but that's harder if the variable must be non-static or is inside a + * structure. This allows exposure under a different name. + */ +#define module_param_hw_named(name, value, type, hwtype, perm) \ + param_check_##type(name, &(value)); \ + __module_param_call(MODULE_PARAM_PREFIX, name, \ + ¶m_ops_##type, &value, \ + perm, -1, \ + KERNEL_PARAM_FL_HWPARAM | (hwparam_##hwtype & 0)); \ + __MODULE_PARM_TYPE(name, #type) + +#define module_param_hw(name, type, hwtype, perm) \ + module_param_hw_named(name, name, type, hwtype, perm) + +/** + * module_param_hw_array - A parameter representing an array of hw parameters + * @name: the name of the array variable + * @type: the type, as per module_param() + * @hwtype: what the value represents (enum hwparam_type) + * @nump: optional pointer filled in with the number written + * @perm: visibility in sysfs + * + * Input and output are as comma-separated values. Commas inside values + * don't work properly (eg. an array of charp). + * + * ARRAY_SIZE(@name) is used to determine the number of elements in the + * array, so the definition must be visible. + */ +#define module_param_hw_array(name, type, hwtype, nump, perm) \ + param_check_##type(name, &(name)[0]); \ + static const struct kparam_array __param_arr_##name \ + = { .max = ARRAY_SIZE(name), .num = nump, \ + .ops = ¶m_ops_##type, \ + .elemsize = sizeof(name[0]), .elem = name }; \ + __module_param_call(MODULE_PARAM_PREFIX, name, \ + ¶m_array_ops, \ + .arr = &__param_arr_##name, \ + perm, -1, \ + KERNEL_PARAM_FL_HWPARAM | (hwparam_##hwtype & 0)); \ + __MODULE_PARM_TYPE(name, "array of " #type) + + extern const struct kernel_param_ops param_array_ops; extern const struct kernel_param_ops param_ops_string;
Provided an annotation for module parameters that specify hardware parameters (such as io ports, iomem addresses, irqs, dma channels, fixed dma buffers and other types). This will enable such parameters to be locked down in the core parameter parser for secure boot support. I've also included annotations as to what sort of hardware configuration each module is dealing with for future use. Some of these are straightforward (ioport, iomem, irq, dma), but there are also: (1) drivers that switch the semantics of a parameter between ioport and iomem depending on a second parameter, (2) drivers that appear to reserve a CPU memory buffer at a fixed address, (3) other parameters, such as bus types and irq selection bitmasks. For the moment, the hardware configuration type isn't actually stored, though its validity is checked. Signed-off-by: David Howells <dhowells@redhat.com> --- include/linux/moduleparam.h | 65 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html