| Message ID | 20240718011504.4106163-2-mstrodl@csh.rit.edu |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Congatec CGEB BIOS interface | expand |
On Wed, Jul 17, 2024 at 09:15:02PM -0400, Mary Strodl wrote: > After the ability to allocate PAGE_KERNEL_EXEC memory was removed > from __vmalloc, Yes. for a very good reason. NAK to a driver creating random writable and exectutable memory: Nacked-by: Christoph Hellwig <hch@lst.de>
On Wed, Jul 17, 2024 at 08:04:01PM -0700, Christoph Hellwig wrote:
> NAK to a driver creating random writable and exectutable memory:
Is there a better way to do what I'm trying to do? Or some way to
expose this functionality with more guard rails so that it's a
little bit safer?
Thank you for taking time to review my patches!
On Thu, Jul 18, 2024 at 01:45:11PM +0100, Matthew Wilcox wrote: > On Thu, Jul 18, 2024 at 08:40:27AM -0400, Mary Strodl wrote: > > On Wed, Jul 17, 2024 at 08:04:01PM -0700, Christoph Hellwig wrote: > > > NAK to a driver creating random writable and exectutable memory: > > > > Is there a better way to do what I'm trying to do? Or some way to > > expose this functionality with more guard rails so that it's a > > little bit safer? > > No, there is no way to do what you're trying to do. IFF it is absolutely required to run BIOS provided executable code, the best way to do that is in a separate userspace process.
On Thu, Jul 18, 2024 at 01:53:23PM +0100, Matthew Wilcox wrote: > That does work, but I would assume that since this BIOS exists to > communicate with the hardware that it'd need various special privileges > and that running in ring 3 would not work. Exactly. > Ultimately, better off running the whole thing inside a VM and passing > the device through to the guest. Or ignoring the BIOS entirely and > implementing direct access to the hardware. But neither of these > approaches is "a way to do what I'm trying to do", they're entirely > different approaches to making this hardware work. If I ran the whole thing inside a VM, I would still need support in the kernel, right? As far as I know, there is no documentation on Congatec's side about the underlying interface. Obviously I could disassemble the blob in the BIOS and figure it out, but I suspect that will have much less hardware compatibility and be subject to random breakage if they make a BIOS update or something. Plus, I would probably run afoul of copyright if I wrote a driver after doing that. I'm not really thrilled that this is their design either, but I'm not sure that there is a better answer... Thank you!
On Thu, 18 Jul 2024 09:20:15 -0400 Mary Strodl <mstrodl@freedom.csh.rit.edu> wrote: > On Thu, Jul 18, 2024 at 01:53:23PM +0100, Matthew Wilcox wrote: > > That does work, but I would assume that since this BIOS exists to > > communicate with the hardware that it'd need various special privileges > > and that running in ring 3 would not work. > > Exactly. > > > Ultimately, better off running the whole thing inside a VM and passing > > the device through to the guest. Or ignoring the BIOS entirely and > > implementing direct access to the hardware. But neither of these > > approaches is "a way to do what I'm trying to do", they're entirely > > different approaches to making this hardware work. > > If I ran the whole thing inside a VM, I would still need support in the > kernel, right? > > As far as I know, there is no documentation on Congatec's side about the > underlying interface. Obviously I could disassemble the blob in the BIOS > and figure it out, but I suspect that will have much less hardware > compatibility and be subject to random breakage if they make a BIOS > update or something. Plus, I would probably run afoul of copyright if I > wrote a driver after doing that. > > I'm not really thrilled that this is their design either, but I'm not > sure that there is a better answer... > The hardware is weird, but we should try to support it in some fashion. But without making dangerous functionality more widely available. So we're looking for some solution which can be fully contained within that hardware's driver. Dumb idea, there will be other ideas: is it practical to take that code blob out of the BIOS, put it into a kernel module (as a .byte table in a .s file and suitable C interfacing), compile that up and insmod that module?
On Thu, 18 Jul 2024 22:35:02 +0100 Matthew Wilcox <willy@infradead.org> wrote: > On Thu, Jul 18, 2024 at 02:31:03PM -0700, Andrew Morton wrote: > > The hardware is weird, but we should try to support it in some fashion. > > Why? It's been around since 2005, and Linux has done perfectly well > without support for it. Oh. I was assuming it was some new thing. This does weaken the case a lot.
> > On Thu, 18 Jul 2024 22:35:02 +0100 Matthew Wilcox <willy@infradead.org> wrote: > > > On Thu, Jul 18, 2024 at 02:31:03PM -0700, Andrew Morton wrote: > > > The hardware is weird, but we should try to support it in some fashion. > > > > Why? It's been around since 2005, and Linux has done perfectly well > > without support for it. > > Oh. I was assuming it was some new thing. This does weaken the case > a lot. This wonderful interface is used in recent products from them too. Adding support for it in an upstream-able way could be still a benefit, as these products are used in different industrial environments running on Linux.
On Fri, Jul 19, 2024 at 09:59:37PM +0200, Rudolf Marek wrote: > I would suggest to simply run the BIOS code of this interface in usermode. Sort of similar to VM86 VESA stuff. > Last time I looked into this it used STI/CLI/RDMSR/WRMSR and couple of I/O ports and cf8/cfc for PCI. I took a look at uvesafb (which appears to be what you were talking about) and it looks like it starts a separate executable and uses some IPC to talk to it. Is that the best way to do it? I guess it would look something like: - driver gets loaded - driver spawns /sbin/cgeb-helper - driver uses cn_netlink_send to send the `high_desc` to helper Then the calls to `board->entry` in the driver get replaced with `cn_netlink_send` with a `cgeb_fps`. When the userspace helper gets the message with the `cgeb_fps`, it calls into the bios code and replies to the driver with send() and passes back cgeb_fps.
On Fri, 19 Jul 2024 13:42:40 +0100 Matthew Wilcox <willy@infradead.org> wrote: > On Fri, Jul 19, 2024 at 07:58:40AM -0400, Mary Strodl wrote: > > Maybe some of the stuff the driver does right now could be moved into > > vmalloc? In other words, we could provide a different function that > > allocates an executable page, copies memory into it, then marks it > > read-only. Would that do better to alleviate concerns? > > No. We are not running arbitrary x86 code. That is a security > nightmare. Sure, if such a thing were to be done we'd want it localized within the driver rather than offered globally. But if there was some hack within the driver to do this, what problems might that cause? What are the scenarios?
On Tue, Jul 23, 2024 at 05:00:43PM -0700, Andrew Morton wrote: > On Fri, 19 Jul 2024 13:42:40 +0100 Matthew Wilcox <willy@infradead.org> wrote: > > > On Fri, Jul 19, 2024 at 07:58:40AM -0400, Mary Strodl wrote: > > > Maybe some of the stuff the driver does right now could be moved into > > > vmalloc? In other words, we could provide a different function that > > > allocates an executable page, copies memory into it, then marks it > > > read-only. Would that do better to alleviate concerns? > > > > No. We are not running arbitrary x86 code. That is a security > > nightmare. > > Sure, if such a thing were to be done we'd want it localized within the > driver rather than offered globally. > > But if there was some hack within the driver to do this, what problems > might that cause? What are the scenarios? That we're running arbitrary x86 code (provided by the manufacturer) inside the kernel where it can undermine every security guarantee we provide?
On Wed, Jul 24, 2024 at 01:16:02AM +0100, Matthew Wilcox wrote: > That we're running arbitrary x86 code (provided by the manufacturer) > inside the kernel where it can undermine every security guarantee we > provide? .. and by exporting the interface allow arbitrary other code including exploit code to allocate writable and executable memory?
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index e34ea860153f..037b7e0fe430 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -3879,6 +3879,7 @@ void *__vmalloc_node_range_noprof(unsigned long size, unsigned long align, return NULL; } +EXPORT_SYMBOL(__vmalloc_node_range_noprof); /** * __vmalloc_node - allocate virtually contiguous memory
After the ability to allocate PAGE_KERNEL_EXEC memory was removed from __vmalloc, this seems like the least invasive way to expose the capability to drivers that need it. Exports __vmalloc_node_range so that drivers can use it. Signed-off-by: Mary Strodl <mstrodl@csh.rit.edu> --- mm/vmalloc.c | 1 + 1 file changed, 1 insertion(+)