Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1587485099.git.daniele.alessandrelli@intel.com>
 <13ca92165fab2827b6d439661e75f5b91ef083c2.1587485099.git.daniele.alessandrelli@intel.com>
 <20200501081002.GA1055721@kroah.com> <f60aece195cd0700728fc38b0398949a82b72fc3.camel@linux.intel.com>
 <20200524212851.GG1192@bug> <CAK8P3a225pqBfzQ19e6Gt0s_tYBp29xLb8EG==hhz=1wc7aVCA@mail.gmail.com>
 <ac0534138facc25c4cbcbbff68fc0ba3c2de87b6.camel@linux.intel.com>
In-Reply-To: <ac0534138facc25c4cbcbbff68fc0ba3c2de87b6.camel@linux.intel.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Wed, 27 May 2020 16:33:58 +0200
Message-ID: <CAK8P3a2MzqF3P8nY3hAUaAXhTV8ZGQd187UDbNV1GBdu+_z5-g@mail.gmail.com>
Subject: Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver
To:     "Alessandrelli, Daniele" <daniele.alessandrelli@intel.com>
Cc:     "pavel@ucw.cz" <pavel@ucw.cz>, "robh@kernel.org" <robh@kernel.org>,
        "Murphy, Paul J" <paul.j.murphy@intel.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "Shevchenko, Andriy" <andriy.shevchenko@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "daniele.alessandrelli@linux.intel.com" 
        <daniele.alessandrelli@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele
<daniele.alessandrelli@intel.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