linux-kernel - Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAK8P3a2MzqF3P8nY3hAUaAXhTV8ZGQd187UDbNV1GBdu+_z5-g@mail.gmail.com>
Date:   Wed, 27 May 2020 16:33:58 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     "Alessandrelli, Daniele" <daniele.alessandrelli@...el.com>
Cc:     "pavel@....cz" <pavel@....cz>, "robh@...nel.org" <robh@...nel.org>,
        "Murphy, Paul J" <paul.j.murphy@...el.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "Shevchenko, Andriy" <andriy.shevchenko@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "daniele.alessandrelli@...ux.intel.com" 
        <daniele.alessandrelli@...ux.intel.com>
Subject: Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver

On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele
<daniele.alessandrelli@...el.com> wrote:
> > > Alternatively, take that memory off the "memory available" maps,
> > > and only re-add it once
> > > it is usable.
> > >
> > > Anything else is dangerous.
>
> That sounds like an interesting solution, thanks!
>
> Do you know any code that I can use as a reference?
>
> Since, the protected memory is just a few megabytes large, I guess that
> in theory we could just have U-Boot mark it as reserved memory and
> forget about it; but if there's a way to re-add it back once in Linux
> that would be great.

Adding it back later on with a loadable device driver should
not be a problem, as this is not a violation of the boot protocol.

> > Agreed, this sounds like an incompatible extension of the boot
> > protocol that we should otherwise not merge.
> >
> > However, there is also a lot of missing information here, and it is
> > always possible they are trying to something for a good reason.
> > As long as the problem that the bootloader is trying to solve is
> > explained well enough in the changelog, we can discuss it to see
> > how it should be done properly.
>
> Apologies, I should have provided more information. Here it is :)
>
> Basically, at boot time U-Boot code and core memory (.text, .data,
> .bss, etc.) is protected by this Isolated Memory Region (IMR) which
> prevents any device or processing units other than the ARM CPU to
> access/modify the memory.
>
> This is done for security reasons, to reduce the risks that a potential
> attacker can use "hijacked" HW devices to interfere with the boot
> process (and break the secure boot flow in place).
>
> Before booting the Kernel, U-Boot sets up a new IMR to protect the
> Kernel image (so that the kernel can benefit of a similar protection)
> and then starts the kernel, delegating to the kernel the task of
> switching off the U-Boot IMR.
>
> U-Boot doesn't turn off its own IMR because doing so would leave a
> (tiny) window in which the boot execution flow is not protected.
>
> If you have any additional questions or feedback, just let me know.

Thank you for the detailed explanation. I've never seen this done
in the Linux boot flow, but I can see how it helps prevent certain
kinds of attacks.

It seems that just reserving the u-boot area and optionally
freeing it later from a driver solves most of your problem.
I have one related question though: if the kernel itself is
protected, does that mean that any driver that does a DMA
to statically allocated memory inside of the kernel is broken
as well? I think this is something that a couple of USB drivers
do, though it is fairly rare. Does u-boot protect both only
the executable sections of the kernel or also data, and does
the hardware block both read and write on the IMR, or just
writes?

        Arnd