Re: [RFC] arm64: extra entries in /proc/iomem for kexec

2018-03-14 Thread AKASHI Takahiro
On Wed, Mar 14, 2018 at 08:39:23AM +, Ard Biesheuvel wrote:
> On 14 March 2018 at 08:29, AKASHI Takahiro  wrote:
> > In the last couples of months, there were some problems reported [1],[2]
> > around arm64 kexec/kdump. Where those phenomenon look different,
> > the root cause would be that kexec/kdump doesn't take into account
> > crucial "reserved" regions of system memory and unintentionally corrupts
> > them.
> >
> > Given that kexec-tools looks for all the information by seeking the file,
> > /proc/iomem, the first step to address said problems is to expand this 
> > file's
> > format so that it will have enough information about system memory and
> > its usage.
> >
> > Attached is my experimental code: With this patch applied, /proc/iomem sees
> > something like the below:
> >
> > (format A)
> > 4000-5871 : System RAM
> >   4008-40f1 : Kernel code
> >   4104-411e8fff : Kernel data
> >   5440-583f : Crash kernel
> >   5859-585e : EFI Resources
> >   5870-5871 : EFI Resources
> > 5872-58b5 : System RAM
> >   5872-58b5 : EFI Resources
> > 58b6-5be3 : System RAM
> >   58b61018-58b61947 : EFI Memory Map
> >   59a7b118-59a7b667 : EFI Configuration Tables
> > 5be4-5bec : System RAM  <== (A-1)
> >   5be4-5bec : EFI Resources
> > 5bed-5bed : System RAM
> > 5bee-5bff : System RAM
> >   5bee-5bff : EFI Resources
> > 5c00-5fff : System RAM
> > 80-ff : PCI Bus :00
> >
> > Meanwhile, the workaround I suggested in [3] gave us a simpler view:
> >
> > (format B)
> > 4000-5871 : System RAM
> >   4008-40f1 : Kernel code
> >   4104-411e9fff : Kernel data
> >   5440-583f : Crash kernel
> >   5859-585e : reserved
> >   5870-5871 : reserved
> > 5872-58b5 : reserved
> > 58b6-5be3 : System RAM
> >   58b61000-58b61fff : reserved
> >   59a7b318-59a7b867 : reserved
> > 5be4-5bec : reserved<== (B-1)
> > 5bed-5bed : System RAM
> > 5bee-5bff : reserved
> > 5c00-5fff : System RAM
> >   5ec0-5edf : reserved
> > 80-ff : PCI Bus :00
> >
> > Here all the regions to be protected are named just "reserved" whether
> > they are NOMAP regions or simply-memblock_reserve'd. They are not very
> > useful for anything but kexec/kdump which knows what they mean.
> >
> > Alternatively, we may want to give them more specific names, based on
> > related efi memory map descriptors and else, that will characterize
> > their contents:
> >
> > (format C)
> > 4000-5871 : System RAM
> >   4008-40f1 : Kernel code
> >   4104-411e9fff : Kernel data
> >   5440-583f : Crash kernel
> >   5859-585e : ACPI Reclaim Memory
> >   5870-5871 : ACPI Reclaim Memory
> > 5872-58b5 : System RAM
> >   5872-5878 : Runtime Data
> >   5879-587d : Runtime Code
> >   587e-5882 : Runtime Data
> >   5883-5887 : Runtime Code
> >   5888-588c : Runtime Data
> >   588d-5891 : Runtime Code
> >   5892-5896 : Runtime Data
> >   5897-589b : Runtime Code
> >   589c-58a5 : Runtime Data
> >   58a6-58ab : Runtime Code
> >   58ac-58b0 : Runtime Data
> >   58b1-58b5 : Runtime Code
> > 58b6-5be3 : System RAM
> >   58b61000-58b61fff : EFI Memory Map
> >   59a7b118-59a7b667 : EFI Memory Attributes Table
> > 5be4-5bec : System RAM
> >   5be4-5bec : Runtime Code
> > 5bed-5bed : System RAM
> > 5bee-5bff : System RAM
> >   5bee-5bff : Runtime Data
> > 5c00-5fff : System RAM
> > 80-ff : PCI Bus :00
> >
> > I once created a patch for this format, but it looks quite noisy and
> > names are a sort of mixture of memory attributes( ACPI Reclaim memory,
> > Conventional Memory, Persistent Memory etc.) vs.
> > function/usages ([Loader|Boot Service|Runtime] Code/Data).
> > (As a matter of fact, (C-1) consists of various ACPI tables.)
> > Anyhow, they seem not so useful for most of other applications.
> >
> > Those observations lead to format A, where some entries with the same
> > attributes are squeezed into a single entry under a simple name if they
> > are neighbouring.
> >
> >
> > So my questions here are:
> >
> > 1. Which format, A, B, or C, is the most appropriate for the moment?
> >or any other suggestions?
> >
> 
> I think some variant of B should be sufficient. The only meaningful
> distinction between these reserved regions at a general level is
> whether they are NOMAP or not, so perhaps we can incorporate that.

I would definitely like to give your opinion the first priority,
but also hear from other guys.

Can you tell my why you think that the distinction, NOMAP or not,
is meaningful?

> As for identifying things like EFI configuration tables: this is a

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-14 Thread Borislav Petkov
On Sat, Mar 10, 2018 at 12:33:35AM +, Prakhya, Sai Praneeth wrote:
> Although the discussions were off my understanding, the present issue
> I see is, (and also the motivation for me to do the patch is) when a
> thread tries to execute any efi_runtime_service() we switch to efi_pgd
> (which doesn't have user space mappings) and all other subsystems in
> kernel aren't aware of this switch. This looks like a perfect case for
> kthread.

That's all fine and good but you need to be prepared and handle properly
an NMI while EFI is running.

I have no clue whether the platform delays delivery of NMIs during EFI
runtime services or whatever happens but you need to have all those
cases covered so that no monkey business happens.

Thx.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread James Bottomley
On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > 
> > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > 
> > > This patch adds the logic for checking the kernel module's hash
> > > base on blacklist. The hash must be generated by sha256 and
> > > enrolled
> > > to dbx/mokx.
> > > 
> > > For example:
> > >   sha256sum sample.ko
> > >   mokutil --mokx --import-hash $HASH_RESULT
> > > 
> > > Whether the signature on ko file is stripped or not, the hash can
> > > be
> > > compared by kernel.
> > 
> > What's the use case for this?  We're already in trouble from the
> > ODMs for the size of dbx and its consumption of the extremely
> > limited variable space, so do we really have a use case for adding
> > module blacklist hashes to the UEFI variables given the space
> > constraints (as in one we can't do any other way)?
> > 
> 
> The dbx is a authenticated variable that it can only be updated by
> manufacturer. The mokx gives a flexible way for distro to revoke a
> key or a signed module. Then we don't need to touch shim or bother
> manufacturer to deliver new db. Currently it doesn't have real use
> case yet. 
> 
> I knew that the NVRAM has limited space. But distro needs a backup
> solution for emergency.

I wasn't asking why the variable, I was asking why the mechanism.

OK, let me try to ask the question in a different way:

Why would the distribution need to blacklist a module in this way?  For
the distro to execute the script to add this blacklist, means the
system is getting automated or manual updates ... can't those updates
just remove the module?

The point is that module sha sums are pretty ephemeral in our model
(they change with every kernel), so it seems to be a mismatch to place
them in a permanent blacklist, particularly when we have very limited
space for that list.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] MODSIGN: do not load mok when secure boot disabled

2018-03-14 Thread joeyli
Hi Ard, 

First! Thanks for your review!

On Tue, Mar 13, 2018 at 05:25:30PM +, Ard Biesheuvel wrote:
> On 13 March 2018 at 10:37, Lee, Chun-Yi  wrote:
> > The mok can not be trusted when the secure boot is disabled. Which
> > means that the kernel embedded certificate is the only trusted key.
> >
> > Due to db/dbx are authenticated variables, they needs manufacturer's
> > KEK for update. So db/dbx are secure when secureboot disabled.
> >
> 
> Did you consider the case where secure boot is not implemented? I
> don't think db/dbx are secure in that case, although perhaps it may
> not matter (a bit more information on the purpose of these patches and
> all the shim lingo etc would be appreciated)
> 

The patch 5 in this series checks that the db/dbx must have
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute. But I agree
with you that kernel should checks the SecureBoot variable must
exist in system. I will add patch to detect it.

> > Cc: David Howells 
> > Cc: Josh Boyer 
> > Cc: James Bottomley 
> > Signed-off-by: "Lee, Chun-Yi" 
> > ---
> >  certs/load_uefi.c | 26 +++---
> >  1 file changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/certs/load_uefi.c b/certs/load_uefi.c
> > index 3d88459..d6de4d0 100644
> > --- a/certs/load_uefi.c
> > +++ b/certs/load_uefi.c
> > @@ -164,17 +164,6 @@ static int __init load_uefi_certs(void)
> > }
> > }
> >
> > -   mok = get_cert_list(L"MokListRT", _var, );
> 
> Which tree does this apply to? My tree doesn't have get_cert_list()
>

This patch set is base on the efi-lock-down and keys-uefi branchs in
David Howells's linux-fs git tree.

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-uefi

Thanks a lot!
Joey Lee 
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] arm64: extra entries in /proc/iomem for kexec

2018-03-14 Thread Ard Biesheuvel
On 14 March 2018 at 08:29, AKASHI Takahiro  wrote:
> In the last couples of months, there were some problems reported [1],[2]
> around arm64 kexec/kdump. Where those phenomenon look different,
> the root cause would be that kexec/kdump doesn't take into account
> crucial "reserved" regions of system memory and unintentionally corrupts
> them.
>
> Given that kexec-tools looks for all the information by seeking the file,
> /proc/iomem, the first step to address said problems is to expand this file's
> format so that it will have enough information about system memory and
> its usage.
>
> Attached is my experimental code: With this patch applied, /proc/iomem sees
> something like the below:
>
> (format A)
> 4000-5871 : System RAM
>   4008-40f1 : Kernel code
>   4104-411e8fff : Kernel data
>   5440-583f : Crash kernel
>   5859-585e : EFI Resources
>   5870-5871 : EFI Resources
> 5872-58b5 : System RAM
>   5872-58b5 : EFI Resources
> 58b6-5be3 : System RAM
>   58b61018-58b61947 : EFI Memory Map
>   59a7b118-59a7b667 : EFI Configuration Tables
> 5be4-5bec : System RAM  <== (A-1)
>   5be4-5bec : EFI Resources
> 5bed-5bed : System RAM
> 5bee-5bff : System RAM
>   5bee-5bff : EFI Resources
> 5c00-5fff : System RAM
> 80-ff : PCI Bus :00
>
> Meanwhile, the workaround I suggested in [3] gave us a simpler view:
>
> (format B)
> 4000-5871 : System RAM
>   4008-40f1 : Kernel code
>   4104-411e9fff : Kernel data
>   5440-583f : Crash kernel
>   5859-585e : reserved
>   5870-5871 : reserved
> 5872-58b5 : reserved
> 58b6-5be3 : System RAM
>   58b61000-58b61fff : reserved
>   59a7b318-59a7b867 : reserved
> 5be4-5bec : reserved<== (B-1)
> 5bed-5bed : System RAM
> 5bee-5bff : reserved
> 5c00-5fff : System RAM
>   5ec0-5edf : reserved
> 80-ff : PCI Bus :00
>
> Here all the regions to be protected are named just "reserved" whether
> they are NOMAP regions or simply-memblock_reserve'd. They are not very
> useful for anything but kexec/kdump which knows what they mean.
>
> Alternatively, we may want to give them more specific names, based on
> related efi memory map descriptors and else, that will characterize
> their contents:
>
> (format C)
> 4000-5871 : System RAM
>   4008-40f1 : Kernel code
>   4104-411e9fff : Kernel data
>   5440-583f : Crash kernel
>   5859-585e : ACPI Reclaim Memory
>   5870-5871 : ACPI Reclaim Memory
> 5872-58b5 : System RAM
>   5872-5878 : Runtime Data
>   5879-587d : Runtime Code
>   587e-5882 : Runtime Data
>   5883-5887 : Runtime Code
>   5888-588c : Runtime Data
>   588d-5891 : Runtime Code
>   5892-5896 : Runtime Data
>   5897-589b : Runtime Code
>   589c-58a5 : Runtime Data
>   58a6-58ab : Runtime Code
>   58ac-58b0 : Runtime Data
>   58b1-58b5 : Runtime Code
> 58b6-5be3 : System RAM
>   58b61000-58b61fff : EFI Memory Map
>   59a7b118-59a7b667 : EFI Memory Attributes Table
> 5be4-5bec : System RAM
>   5be4-5bec : Runtime Code
> 5bed-5bed : System RAM
> 5bee-5bff : System RAM
>   5bee-5bff : Runtime Data
> 5c00-5fff : System RAM
> 80-ff : PCI Bus :00
>
> I once created a patch for this format, but it looks quite noisy and
> names are a sort of mixture of memory attributes( ACPI Reclaim memory,
> Conventional Memory, Persistent Memory etc.) vs.
> function/usages ([Loader|Boot Service|Runtime] Code/Data).
> (As a matter of fact, (C-1) consists of various ACPI tables.)
> Anyhow, they seem not so useful for most of other applications.
>
> Those observations lead to format A, where some entries with the same
> attributes are squeezed into a single entry under a simple name if they
> are neighbouring.
>
>
> So my questions here are:
>
> 1. Which format, A, B, or C, is the most appropriate for the moment?
>or any other suggestions?
>

I think some variant of B should be sufficient. The only meaningful
distinction between these reserved regions at a general level is
whether they are NOMAP or not, so perhaps we can incorporate that.

As for identifying things like EFI configuration tables: this is a
moving target, and we also define our own config tables for the TPM
log, screeninfo on ARM etc. Also, for EFI memory types, you can boot
with efi=debug and look at the entire memory map. So I think adding
all that information may be overkill.

> Currently, there is a inconsistent view between (A) and the mainline's:
> see (A-1) and (B-1). If this is really a matter, I can fix it.
> Kexec-tools can be easily modified to accept both formats, though.
>
>
> 2. How should we 

[RFC] arm64: extra entries in /proc/iomem for kexec

2018-03-14 Thread AKASHI Takahiro
In the last couples of months, there were some problems reported [1],[2]
around arm64 kexec/kdump. Where those phenomenon look different,
the root cause would be that kexec/kdump doesn't take into account
crucial "reserved" regions of system memory and unintentionally corrupts
them.

Given that kexec-tools looks for all the information by seeking the file,
/proc/iomem, the first step to address said problems is to expand this file's
format so that it will have enough information about system memory and
its usage.

Attached is my experimental code: With this patch applied, /proc/iomem sees
something like the below:

(format A)
4000-5871 : System RAM
  4008-40f1 : Kernel code
  4104-411e8fff : Kernel data
  5440-583f : Crash kernel
  5859-585e : EFI Resources
  5870-5871 : EFI Resources
5872-58b5 : System RAM
  5872-58b5 : EFI Resources
58b6-5be3 : System RAM
  58b61018-58b61947 : EFI Memory Map
  59a7b118-59a7b667 : EFI Configuration Tables
5be4-5bec : System RAM  <== (A-1)
  5be4-5bec : EFI Resources
5bed-5bed : System RAM
5bee-5bff : System RAM
  5bee-5bff : EFI Resources
5c00-5fff : System RAM
80-ff : PCI Bus :00

Meanwhile, the workaround I suggested in [3] gave us a simpler view:

(format B)
4000-5871 : System RAM
  4008-40f1 : Kernel code
  4104-411e9fff : Kernel data
  5440-583f : Crash kernel
  5859-585e : reserved
  5870-5871 : reserved
5872-58b5 : reserved
58b6-5be3 : System RAM
  58b61000-58b61fff : reserved
  59a7b318-59a7b867 : reserved
5be4-5bec : reserved<== (B-1)
5bed-5bed : System RAM
5bee-5bff : reserved
5c00-5fff : System RAM
  5ec0-5edf : reserved
80-ff : PCI Bus :00

Here all the regions to be protected are named just "reserved" whether
they are NOMAP regions or simply-memblock_reserve'd. They are not very
useful for anything but kexec/kdump which knows what they mean.

Alternatively, we may want to give them more specific names, based on
related efi memory map descriptors and else, that will characterize
their contents:

(format C)
4000-5871 : System RAM
  4008-40f1 : Kernel code
  4104-411e9fff : Kernel data
  5440-583f : Crash kernel
  5859-585e : ACPI Reclaim Memory
  5870-5871 : ACPI Reclaim Memory
5872-58b5 : System RAM
  5872-5878 : Runtime Data
  5879-587d : Runtime Code
  587e-5882 : Runtime Data
  5883-5887 : Runtime Code
  5888-588c : Runtime Data
  588d-5891 : Runtime Code
  5892-5896 : Runtime Data
  5897-589b : Runtime Code
  589c-58a5 : Runtime Data
  58a6-58ab : Runtime Code
  58ac-58b0 : Runtime Data
  58b1-58b5 : Runtime Code
58b6-5be3 : System RAM
  58b61000-58b61fff : EFI Memory Map
  59a7b118-59a7b667 : EFI Memory Attributes Table
5be4-5bec : System RAM
  5be4-5bec : Runtime Code
5bed-5bed : System RAM
5bee-5bff : System RAM
  5bee-5bff : Runtime Data
5c00-5fff : System RAM
80-ff : PCI Bus :00

I once created a patch for this format, but it looks quite noisy and
names are a sort of mixture of memory attributes( ACPI Reclaim memory,
Conventional Memory, Persistent Memory etc.) vs.
function/usages ([Loader|Boot Service|Runtime] Code/Data).
(As a matter of fact, (C-1) consists of various ACPI tables.)
Anyhow, they seem not so useful for most of other applications.

Those observations lead to format A, where some entries with the same
attributes are squeezed into a single entry under a simple name if they
are neighbouring.


So my questions here are:

1. Which format, A, B, or C, is the most appropriate for the moment?
   or any other suggestions?

Currently, there is a inconsistent view between (A) and the mainline's:
see (A-1) and (B-1). If this is really a matter, I can fix it.
Kexec-tools can be easily modified to accept both formats, though.


2. How should we determine which regions be exported in /proc/iomem?

 a. Trust all the memblock_reserve'd regions as my previous patch [3] does.

As I said, it's a kind of "overkill." Some of regions, say fdt, are
not required to be preserved across kexec.

 b. List regions separately and exhaustively later on at a single point
as my patch attached does.

I'm afraid of any possibility that some regions may be doubly counted,
one from efi memory map search and another from other efi/acpi code
(various type of "tables" in most cases).

 c. Expand efi_mem_reserve() with an argument of "resource descriptor" and
replace memblock_reserve() in efi code wherever necessary so as to
maintain an export list.

efi_mem_reserve() was first introduced for specific needs at kexec
on x86, but I 

Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread joeyli
On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for checking the kernel module's hash
> > base on blacklist. The hash must be generated by sha256 and enrolled
> > to dbx/mokx.
> > 
> > For example:
> > sha256sum sample.ko
> > mokutil --mokx --import-hash $HASH_RESULT
> > 
> > Whether the signature on ko file is stripped or not, the hash can be
> > compared by kernel.
> 
> What's the use case for this?  We're already in trouble from the ODMs
> for the size of dbx and its consumption of the extremely limited
> variable space, so do we really have a use case for adding module
> blacklist hashes to the UEFI variables given the space constraints (as
> in one we can't do any other way)?
>

The dbx is a authenticated variable that it can only be updated by
manufacturer. The mokx gives a flexible way for distro to revoke a key
or a signed module. Then we don't need to touch shim or bother
manufacturer to deliver new db. Currently it doesn't have real use
case yet. 

I knew that the NVRAM has limited space. But distro needs a backup
solution for emergency.

Thanks a lot!
Joey Lee 
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html