lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver
From
Date
On Wed, 2020-05-27 at 20:59 +0200, Arnd Bergmann wrote:
> On Wed, May 27, 2020 at 7:43 PM Daniele Alessandrelli
> <daniele.alessandrelli@linux.intel.com> wrote:
> > On Wed, 2020-05-27 at 16:33 +0200, Arnd Bergmann wrote:
> > > On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele <
> > > daniele.alessandrelli@intel.com> wrote:
> > >
> > > 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.
> >
> > Cool, I'll try to do that then, thanks!
> >
> > I see two ways to do that though:
> >
> > 1. Create a device driver that gets a reference to the memory
> > region
> > from its DT node and then re-adds the memory to the pool of
> > available
> > memory.
> >
> > 2. Use a special "compatible" string for my memory region and
> > create a
> > driver to handle it.
>
> I think the first approach is more common.
>
> > However, I think that in the second case the driver must be
> > builtin.
> > Would that be okay?
>
> It's better to avoid that.
>
> > Also, from a quick look, it seems that I can re-add that memory
> > back by
> > calling memblock_free() (or a similar memblock_* function). Am I on
> > the
> > right track?
>
> I'm not sure if memblock_free() works after early memory
> initialization
> is complete, but I think there is some way to do it later. Maybe try
> memblock_free() first, and then look for something else if it doesn't
> work.
>

Brilliant. I will create a new patch using the 1st approach and see if
memblock_free() works; if not, I will look for something else.

Thanks a lot for your valuable feedback and advice.

> > > 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?
> >
> > Yes, you're very right. Drivers that do DMA transfers to statically
> > allocated memory inside the kernel will be broken.
> >
> > We are currently seeing this with our eMMC driver.
> >
> > Current solution is to add the eMMC controller to the list of
> > allowed
> > "agents" for the IMR. This will reduce the level of protection, but
> > since we expect to have only a few of these exceptions (since, as
> > you
> > pointed out, drivers that do DMA to static kernel memory seem to be
> > quite rare), we think that there is still value in having the
> > Kernel
> > IMR.
> >
> > Do you see any issue with this?
>
> I think you should try to fix the driver rather than making an
> exception for it.

Yes, we'll look into that.

> Hot-pluggable drivers are a much more interesting
> case I think, because on the one hand you have no idea what
> users might want to plug in legitimately, but on the other hand
> those are also the most likely attack vectors (driver bugs for
> random USB drivers overwriting kernel memory when faced with
> malicious hardware) that this feature is trying to prevent.
>
> I also wonder whether we should do something in the normal
> iommu code that prevents one from mapping a page that the
> kernel would consider as protected (kernel .text, freed memory,
> ...) into the iommu in the first place.

Sounds like an iteresting security feature; expecially because it would
apply to any hardware.

>
> Arnd



\
 
 \ /
  Last update: 2020-05-28 14:29    [W:0.049 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site