Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 02, 2019 at 11:03:08AM -0700, Matthew Garrett wrote:
> On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
> wrote:
> >
> > On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Not late. This is not part of any PR yet. Thank you for the
> > feedback!
> >
> > Matthew, can you send an updated version of the whole patch set
> > with fixes to this issue and also reordering of the includes?
>
> Yes, I'll resend and let's do this again for 5.3.
Agreed, better not rush but get it right once.
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 02, 2019 at 09:14:49AM +0200, Ard Biesheuvel wrote:
> (+ Ingo)
>
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
>
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
>
> What is currently queued there? Can we revert this change for the time
> being, and resubmit it via the EFI tree for v5.3?
Nothing ATM. The broken code is in my master branch ATM. Nothing
in my next branch nor have I included anything to my PRs. Should
be nothing to worry about in that sense :-)
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
Sorry, how about this one? I was confused by why I wasn't hitting
this, but on closer examination it turns out that my system populates
the final event log with 0 events which means we never hit this
codepath :(
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 2ccaa6661aaf..db0fdaa9c666 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
if (event_size == 0)
return -1;
size += event_size;
+ count--;
}
return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
struct linux_efi_tpm_eventlog *log_tbl;
struct efi_tcg2_final_events_table *final_tbl;
unsigned int tbl_size;
+ int ret = 0;
if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
tbl_size = sizeof(*log_tbl) + log_tbl->size;
memblock_reserve(efi.tpm_log, tbl_size);
- early_memunmap(log_tbl, sizeof(*log_tbl));
if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
- return 0;
+ goto out;
final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
@@ -71,17 +72,20 @@ int __init efi_tpm_eventlog_init(void)
pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
efi.tpm_final_log);
efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out;
}
tbl_size = tpm2_calc_event_log_size(final_tbl->events,
final_tbl->nr_events,
- (void *)efi.tpm_log);
+ log_tbl->log);
memblock_reserve((unsigned long)final_tbl,
tbl_size + sizeof(*final_tbl));
early_memunmap(final_tbl, sizeof(*final_tbl));
efi_tpm_final_log_size = tbl_size;
- return 0;
+out:
+ early_memunmap(log_tbl, sizeof(*log_tbl));
+ return ret;
}
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index dccc97e6135c..190a33968a91 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -158,7 +158,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
{
struct tcg_efi_specid_event_head *efispecid;
struct tcg_event_field *event_field;
- void *mapping = NULL;
int mapping_size;
void *marker;
void *marker_start;
@@ -176,9 +175,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
/* Map the event header */
if (do_mapping) {
mapping_size = marker - marker_start;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -199,9 +198,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
if (do_mapping) {
early_memunmap(mapping, mapping_size);
mapping_size = marker - marker_start + halg_size;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -219,9 +218,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
if (do_mapping) {
early_memunmap(mapping, mapping_size);
mapping_size = marker - marker_start;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -243,11 +242,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
* we don't need to map it
*/
if (do_mapping) {
- early_memunmap(marker_start, mapping_size);
+ early_memunmap(event, mapping_size);
mapping_size += sizeof(event_field->event_size);
- mapping = early_memremap((unsigned long)marker_start,
- mapping_size);
- if (!mapping) {
+ event = early_memremap((unsigned long)marker_start,
+ mapping_size);
+ if (!event) {
size = 0;
goto out;
}
@@ -257,8 +256,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
+ event_field->event_size;
size = marker - marker_start;
- if ((event->event_type == 0) && (event_field->event_size == 0))
- size = 0;
out:
if (do_mapping)
early_memunmap(mapping, mapping_size);
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
wrote:
>
> (+ Ingo)
>
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
>
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
The patchset was Cc:ed to linux-efi@. Is there anything else I should
have done to ensure you picked it up rather than Jarkko?
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
wrote:
>
> On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Not late. This is not part of any PR yet. Thank you for the
> feedback!
>
> Matthew, can you send an updated version of the whole patch set
> with fixes to this issue and also reordering of the includes?
Yes, I'll resend and let's do this again for 5.3.
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, 2 May 2019 06:46:23 -0700
Matthew Wilcox wrote:
> On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote:
> > Drop the pgtable_t variable from all implementation for pte_fn_t as none of
> > them use it. apply_to_pte_range() should stop computing it as well. Should
> > help us save some cycles.
>
> You didn't add Martin Schwidefsky for some reason. He introduced
> it originally for s390 for sub-page page tables back in 2008 (commit
> 2f569afd9c). I think he should confirm that he no longer needs it.
With its 2K pte tables s390 can not deal with a (struct page *) as a reference
to a page table. But if there are no user of the apply_to_page_range() API
left which actually make use of the token argument we can safely drop it.
> > ---
> > - Boot tested on arm64 and x86 platforms.
> > - Build tested on multiple platforms with their defconfig
> >
> > arch/arm/kernel/efi.c | 3 +--
> > arch/arm/mm/dma-mapping.c | 3 +--
> > arch/arm/mm/pageattr.c | 3 +--
> > arch/arm64/kernel/efi.c| 3 +--
> > arch/arm64/mm/pageattr.c | 3 +--
> > arch/x86/xen/mmu_pv.c | 3 +--
> > drivers/gpu/drm/i915/i915_mm.c | 3 +--
> > drivers/xen/gntdev.c | 6 ++
> > drivers/xen/privcmd.c | 6 ++
> > drivers/xen/xlate_mmu.c| 3 +--
> > include/linux/mm.h | 3 +--
> > mm/memory.c| 5 +
> > mm/vmalloc.c | 2 +-
> > 13 files changed, 15 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> > index 9f43ba012d10..b1f142a01f2f 100644
> > --- a/arch/arm/kernel/efi.c
> > +++ b/arch/arm/kernel/efi.c
> > @@ -11,8 +11,7 @@
> > #include
> > #include
> >
> > -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> > - unsigned long addr, void *data)
> > +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > efi_memory_desc_t *md = data;
> > pte_t pte = *ptep;
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 43f46aa7ef33..739286511a18 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
> > }
> > }
> >
> > -static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
> > {
> > struct page *page = virt_to_page(addr);
> > pgprot_t prot = *(pgprot_t *)data;
> > diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> > index 1403cb4a0c3d..c8b500940e1f 100644
> > --- a/arch/arm/mm/pageattr.c
> > +++ b/arch/arm/mm/pageattr.c
> > @@ -22,8 +22,7 @@ struct page_change_data {
> > pgprot_t clear_mask;
> > };
> >
> > -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> > {
> > struct page_change_data *cdata = data;
> > pte_t pte = *ptep;
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 4f9acb5fbe97..230cff073a08 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
> > efi_memory_desc_t *md)
> > return 0;
> > }
> >
> > -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> > - unsigned long addr, void *data)
> > +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > efi_memory_desc_t *md = data;
> > pte_t pte = READ_ONCE(*ptep);
> > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> > index 6cd645edcf35..0be077628b21 100644
> > --- a/arch/arm64/mm/pageattr.c
> > +++ b/arch/arm64/mm/pageattr.c
> > @@ -27,8 +27,7 @@ struct page_change_data {
> >
> > bool rodata_full __ro_after_init =
> > IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
> >
> > -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> > {
> > struct page_change_data *cdata = data;
> > pte_t pte = READ_ONCE(*ptep);
> > diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> > index a21e1734fc1f..308a6195fd26 100644
> > --- a/arch/x86/xen/mmu_pv.c
> > +++ b/arch/x86/xen/mmu_pv.c
> > @@ -2702,8 +2702,7 @@ struct remap_data {
> > struct mmu_update *mmu_update;
> > };
> >
> > -static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
> > -unsigned long addr, void *data)
> > +static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > struct remap_data *rmd = data;
> > pte_t pte =
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On 05/02/2019 07:16 PM, Matthew Wilcox wrote: > On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote: >> Drop the pgtable_t variable from all implementation for pte_fn_t as none of >> them use it. apply_to_pte_range() should stop computing it as well. Should >> help us save some cycles. > You didn't add Martin Schwidefsky for some reason. He introduced scripts/get_maintainer.pl did not list the email but anyways I should have added it from git blame. Thanks for adding his email to the thread.
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote:
> Drop the pgtable_t variable from all implementation for pte_fn_t as none of
> them use it. apply_to_pte_range() should stop computing it as well. Should
> help us save some cycles.
You didn't add Martin Schwidefsky for some reason. He introduced
it originally for s390 for sub-page page tables back in 2008 (commit
2f569afd9c). I think he should confirm that he no longer needs it.
> ---
> - Boot tested on arm64 and x86 platforms.
> - Build tested on multiple platforms with their defconfig
>
> arch/arm/kernel/efi.c | 3 +--
> arch/arm/mm/dma-mapping.c | 3 +--
> arch/arm/mm/pageattr.c | 3 +--
> arch/arm64/kernel/efi.c| 3 +--
> arch/arm64/mm/pageattr.c | 3 +--
> arch/x86/xen/mmu_pv.c | 3 +--
> drivers/gpu/drm/i915/i915_mm.c | 3 +--
> drivers/xen/gntdev.c | 6 ++
> drivers/xen/privcmd.c | 6 ++
> drivers/xen/xlate_mmu.c| 3 +--
> include/linux/mm.h | 3 +--
> mm/memory.c| 5 +
> mm/vmalloc.c | 2 +-
> 13 files changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> index 9f43ba012d10..b1f142a01f2f 100644
> --- a/arch/arm/kernel/efi.c
> +++ b/arch/arm/kernel/efi.c
> @@ -11,8 +11,7 @@
> #include
> #include
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 43f46aa7ef33..739286511a18 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
> }
> }
>
> -static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long addr,
> - void *data)
> +static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
> {
> struct page *page = virt_to_page(addr);
> pgprot_t prot = *(pgprot_t *)data;
> diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> index 1403cb4a0c3d..c8b500940e1f 100644
> --- a/arch/arm/mm/pageattr.c
> +++ b/arch/arm/mm/pageattr.c
> @@ -22,8 +22,7 @@ struct page_change_data {
> pgprot_t clear_mask;
> };
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 4f9acb5fbe97..230cff073a08 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
> efi_memory_desc_t *md)
> return 0;
> }
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 6cd645edcf35..0be077628b21 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -27,8 +27,7 @@ struct page_change_data {
>
> bool rodata_full __ro_after_init =
> IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index a21e1734fc1f..308a6195fd26 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2702,8 +2702,7 @@ struct remap_data {
> struct mmu_update *mmu_update;
> };
>
> -static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void *data)
> {
> struct remap_data *rmd = data;
> pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot));
> diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
> index e4935dd1fd37..c23bb29e6d3e 100644
> --- a/drivers/gpu/drm/i915/i915_mm.c
> +++ b/drivers/gpu/drm/i915/i915_mm.c
> @@ -35,8 +35,7 @@ struct remap_pfn {
> pgprot_t prot;
> };
>
> -static int remap_pfn(pte_t *pte, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
> {
>
[PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
Drop the pgtable_t variable from all implementation for pte_fn_t as none of
them use it. apply_to_pte_range() should stop computing it as well. Should
help us save some cycles.
Signed-off-by: Anshuman Khandual
Cc: Ard Biesheuvel
Cc: Russell King
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Andrew Morton
Cc: Michal Hocko
Cc: Matthew Wilcox
Cc: Logan Gunthorpe
Cc: "Kirill A. Shutemov"
Cc: Dan Williams
Cc:
Cc: Mike Rapoport
Cc: x...@kernel.org
Cc: linux-efi@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Cc: intel-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux...@kvack.org
---
- Boot tested on arm64 and x86 platforms.
- Build tested on multiple platforms with their defconfig
arch/arm/kernel/efi.c | 3 +--
arch/arm/mm/dma-mapping.c | 3 +--
arch/arm/mm/pageattr.c | 3 +--
arch/arm64/kernel/efi.c| 3 +--
arch/arm64/mm/pageattr.c | 3 +--
arch/x86/xen/mmu_pv.c | 3 +--
drivers/gpu/drm/i915/i915_mm.c | 3 +--
drivers/xen/gntdev.c | 6 ++
drivers/xen/privcmd.c | 6 ++
drivers/xen/xlate_mmu.c| 3 +--
include/linux/mm.h | 3 +--
mm/memory.c| 5 +
mm/vmalloc.c | 2 +-
13 files changed, 15 insertions(+), 31 deletions(-)
diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
index 9f43ba012d10..b1f142a01f2f 100644
--- a/arch/arm/kernel/efi.c
+++ b/arch/arm/kernel/efi.c
@@ -11,8 +11,7 @@
#include
#include
-static int __init set_permissions(pte_t *ptep, pgtable_t token,
- unsigned long addr, void *data)
+static int __init set_permissions(pte_t *ptep, unsigned long addr, void *data)
{
efi_memory_desc_t *md = data;
pte_t pte = *ptep;
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 43f46aa7ef33..739286511a18 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
}
}
-static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long addr,
- void *data)
+static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
{
struct page *page = virt_to_page(addr);
pgprot_t prot = *(pgprot_t *)data;
diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
index 1403cb4a0c3d..c8b500940e1f 100644
--- a/arch/arm/mm/pageattr.c
+++ b/arch/arm/mm/pageattr.c
@@ -22,8 +22,7 @@ struct page_change_data {
pgprot_t clear_mask;
};
-static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
- void *data)
+static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
{
struct page_change_data *cdata = data;
pte_t pte = *ptep;
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4f9acb5fbe97..230cff073a08 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
efi_memory_desc_t *md)
return 0;
}
-static int __init set_permissions(pte_t *ptep, pgtable_t token,
- unsigned long addr, void *data)
+static int __init set_permissions(pte_t *ptep, unsigned long addr, void *data)
{
efi_memory_desc_t *md = data;
pte_t pte = READ_ONCE(*ptep);
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 6cd645edcf35..0be077628b21 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -27,8 +27,7 @@ struct page_change_data {
bool rodata_full __ro_after_init =
IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
-static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
- void *data)
+static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
{
struct page_change_data *cdata = data;
pte_t pte = READ_ONCE(*ptep);
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index a21e1734fc1f..308a6195fd26 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2702,8 +2702,7 @@ struct remap_data {
struct mmu_update *mmu_update;
};
-static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
-unsigned long addr, void *data)
+static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void *data)
{
struct remap_data *rmd = data;
pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot));
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index e4935dd1fd37..c23bb29e6d3e 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -35,8 +35,7 @@ struct remap_pfn {
pgprot_t prot;
};
-static int remap_pfn(pte_t *pte, pgtable_t token,
-unsigned long addr, void *data)
Re: [PATCH 2/2 v3] efi: print appropriate status message when loading certificates
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote:
>
> When loading certificates list from UEFI variable, the original error
> message direct shows the efi status code from UEFI firmware. It looks
> ugly:
>
> [2.335031] Couldn't get size: 0x800e
> [2.335032] Couldn't get UEFI MokListRT
> [2.339985] Couldn't get size: 0x800e
> [2.339987] Couldn't get UEFI dbx list
>
> So, this patch shows the status string instead of status code.
>
> On the other hand, the "Couldn't get UEFI" message doesn't need
> to be exposed when db/dbx/mok variable do not exist. So, this
> patch set the message level to debug.
>
> v3.
> - Print messages similar to db/mok when loading dbx hash to blacklist:
> [1.500952] EFI: Blacklisting hash of an executable: UEFI:dbx
> [1.501773] blacklist: Loaded blacklisting hash
> 'bin:80b4d96931bf0d02fd91a61e19d14f1da452e66db2408ca8604d411f92659f0a'
>
> - Setting messages for the existence of db/mok/dbx lists to debug level.
>
> v2.
> Setting the MODSIGN messages level to debug.
>
> Link:
> https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
> Cc: James Morris
> Cc: Serge E. Hallyn"
> Cc: David Howells
> Cc: Nayna Jain
> Cc: Josh Boyer
> Cc: Mimi Zohar
> Signed-off-by: "Lee, Chun-Yi"
> ---
> certs/blacklist.c | 3 +-
> security/integrity/platform_certs/load_uefi.c | 40
> +++
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 3a507b9e2568..f91437e39e44 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -100,7 +100,8 @@ int mark_hash_blacklisted(const char *hash)
> if (IS_ERR(key)) {
> pr_err("Problem blacklisting hash (%ld)\n", PTR_ERR(key));
> return PTR_ERR(key);
> - }
> + } else
> + pr_notice("Loaded blacklisting hash '%s'\n", hash);
> return 0;
> }
>
> diff --git a/security/integrity/platform_certs/load_uefi.c
> b/security/integrity/platform_certs/load_uefi.c
> index 81b19c52832b..6b6996e5bc27 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#define pr_fmt(fmt) "EFI: "fmt
> +
> #include
> #include
> #include
> @@ -35,6 +37,18 @@ static __init bool uefi_check_ignore_db(void)
> return status == EFI_SUCCESS;
> }
>
> +static void str16_to_str(efi_char16_t *str16, char *str, int str_size)
> +{
> + int i = 0;
> +
> + while (str16[i] != '\0' && i < (str_size - 1)) {
> + str[i] = str16[i];
> + i++;
> + }
> +
> + str[i] = '\0';
> +}
> +
> /*
> * Get a certificate list blob from the named EFI variable.
> */
> @@ -44,13 +58,20 @@ static __init void *get_cert_list(efi_char16_t *name,
> efi_guid_t *guid,
> efi_status_t status;
> unsigned long lsize = 4;
> unsigned long tmpdb[4];
> + char namestr[16];
> void *db;
>
> + str16_to_str(name, namestr, ARRAY_SIZE(namestr));
Please drop this (and the function above) - instead, just return NULL
if the variable is not found (without reporting an error).
> status = efi.get_variable(name, guid, NULL, , );
> if (status != EFI_BUFFER_TOO_SMALL) {
> - pr_err("Couldn't get size: 0x%lx\n", status);
> + if (status == EFI_NOT_FOUND)
> + pr_debug("UEFI %s list doesn't exist\n", namestr);
> + else
> + pr_err("Couldn't get size for UEFI %s list: %s\n",
> + namestr, efi_status_to_str(status));
> return NULL;
> }
> + pr_debug("UEFI %s list exists\n", namestr);
>
> db = kmalloc(lsize, GFP_KERNEL);
> if (!db)
> @@ -59,7 +80,8 @@ static __init void *get_cert_list(efi_char16_t *name,
> efi_guid_t *guid,
> status = efi.get_variable(name, guid, NULL, , db);
> if (status != EFI_SUCCESS) {
> kfree(db);
> - pr_err("Error reading db var: 0x%lx\n", status);
> + pr_err("Error reading UEFI %s list: %s\n",
> + namestr, efi_status_to_str(status));
> return NULL;
> }
>
> @@ -95,6 +117,7 @@ static __init void uefi_blacklist_hash(const char *source,
> const void *data,
> static __init void uefi_blacklist_x509_tbs(const char *source,
>const void *data, size_t len)
> {
> + pr_info("Blacklisting X.509 TBS hash: %s\n", source);
> uefi_blacklist_hash(source, data, len, "tbs:", 4);
> }
>
> @@ -104,6 +127,7 @@ static __init void uefi_blacklist_x509_tbs(const char
> *source,
> static __init void uefi_blacklist_binary(const char *source,
> const void *data, size_t len)
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote:
>
> This function can be used to convert EFI status value to string
> for printing out debug message. Using this function can improve
> the readability of log.
>
> v2.
Please move the changelog out of the commit log (move it below the ---
further down)
> - Changed the wording in subject and description.
> - Moved the marco immediately after the status value definitions.
> - Turned into a proper function instead of inline.
>
You missed my point here. A proper function means the function in a .c
file, and only the declaration in a .h file. This way, you are still
duplicating the literal strings into every object file that references
this function.
> Cc: Ard Biesheuvel
> Cc: Kees Cook
> Cc: Anton Vorontsov
> Cc: Colin Cross
> Cc: Tony Luck
> Signed-off-by: "Lee, Chun-Yi"
> ---
> include/linux/efi.h | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 54357a258b35..6f3f89a32eef 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -42,6 +42,34 @@
> #define EFI_ABORTED(21 | (1UL << (BITS_PER_LONG-1)))
> #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> + return "EFI_" __stringify(_status);
> +
> +static __attribute__((unused)) char *
> +efi_status_to_str(unsigned long status)
> +{
> + switch (status) {
> + EFI_STATUS_STR(SUCCESS)
> + EFI_STATUS_STR(LOAD_ERROR)
> + EFI_STATUS_STR(INVALID_PARAMETER)
> + EFI_STATUS_STR(UNSUPPORTED)
> + EFI_STATUS_STR(BAD_BUFFER_SIZE)
> + EFI_STATUS_STR(BUFFER_TOO_SMALL)
> + EFI_STATUS_STR(NOT_READY)
> + EFI_STATUS_STR(DEVICE_ERROR)
> + EFI_STATUS_STR(WRITE_PROTECTED)
> + EFI_STATUS_STR(OUT_OF_RESOURCES)
> + EFI_STATUS_STR(NOT_FOUND)
> + EFI_STATUS_STR(ABORTED)
> + EFI_STATUS_STR(SECURITY_VIOLATION)
> + default:
> + pr_warn("Unknown efi status: 0x%lx", status);
> + }
> +
> + return "Unknown efi status";
> +}
> +
> typedef unsigned long efi_status_t;
> typedef u8 efi_bool_t;
> typedef u16 efi_char16_t; /* UNICODE character */
> --
> 2.16.4
>
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> I may be a little late with this comment, but I've just tested these
> patches on aarch64 platform (from the top of jjs/master) and got
> kernel panic ("Unable to handle kernel read", full log at the end of
> mail). I think there's problem with below call to
> tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> passed as (void *) and never remapped:
Not late. This is not part of any PR yet. Thank you for the
feedback!
Matthew, can you send an updated version of the whole patch set
with fixes to this issue and also reordering of the includes?
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
(+ Ingo)
On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
>
> On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek wrote:
> >
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Yes, it looks like this is just broken. Can you try with the attached patch?
I'm a bit uncomfortable with EFI code that is obviously broken and
untested being queued for the next merge window in another tree.
What is currently queued there? Can we revert this change for the time
being, and resubmit it via the EFI tree for v5.3?
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
Second patch tries to unmap "mapping" which is not declared. I'm on top of jjs/master and your TPM_MEMREMAP patches are already there, so the first patch applied cleanly. Using it, kernel still panicked on boot: EFI stub: Booting Linux Kernel... EFI stub: EFI_RNG_PROTOCOL unavailable, no randomness supplied EFI stub: Using DTB from configuration table EFI stub: Exiting boot services and installing virtual address map... [0.00] Booting Linux on physical CPU 0x00 [0x420f5162] [0.00] Linux version 5.1.0-rc2+ (root@localhost.localdomain) (gcc version 7.3.1 20180712 (Red Hat 7.3.1-6) (GCC)) #78 SMP Wed May 1 01:05:38 EDT 2019 [0.00] earlycon: pl11 at MMIO 0x00040202 (options '115200n8') [0.00] printk: bootconsole [pl11] enabled [0.00] efi: Getting EFI parameters from FDT: [0.00] efi: EFI v2.60 by Cavium Inc. TX2-FW-Release-7.2-build_08-0-g14f8c5bf8a Apr 15 2019 18:51:41 [0.00] efi: TPMFinalLog=0xed5f SMBIOS=0xfad9 SMBIOS 3.0=0xed53 ACPI 2.0=0xeda9 ESRT=0xfafdb218 MEMATTR=0xf8489018 TPMEventLog=0xedaa9018 MEMRESERVE=0xedaa8018 [0.00] Unhandled fault at 0x7dfffe77a018 [0.00] Mem abort info: [0.00] ESR = 0x9603 [0.00] Exception class = DABT (current EL), IL = 32 bits [0.00] SET = 0, FnV = 0 [0.00] EA = 0, S1PTW = 0 [0.00] Data abort info: [0.00] ISV = 0, ISS = 0x0003 [0.00] CM = 0, WnR = 0 [0.00] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (ptrval) [0.00] [7dfffe77a018] pgd=81b12003 [0.00] [ cut here ] [0.00] kernel BUG at arch/arm64/mm/fault.c:189! [0.00] Internal error: Oops - BUG: 0 [#1] SMP [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 5.1.0-rc2+ #78 [0.00] pstate: 60400089 (nZCv daIf +PAN -UAO) [0.00] pc : show_pte+0x1d0/0x1f0 [0.00] lr : show_pte+0x88/0x1f0 [0.00] sp : 11533b30 [0.00] x29: 11533b30 x28: 115473c0 [0.00] x27: 11542500 x26: 7dfffe73a010 [0.00] x25: 0018 x24: 0025 [0.00] x23: 00fb x22: 117fc000 [0.00] x21: 10f32000 x20: [0.00] x19: 7dfffe77a018 x18: [0.00] x17: x16: [0.00] x15: 1153d708 x14: 1172f420 [0.00] x13: 1172f069 x12: 11568000 [0.00] x11: 11533800 x10: 11533800 [0.00] x9 : 1153ef58 x8 : 303030303030303d [0.00] x7 : 646770205d383130 x6 : 1172e7ff [0.00] x5 : x4 : [0.00] x3 : x2 : [0.00] x1 : 81b12000 x0 : 0ff8 [0.00] Process swapper (pid: 0, stack limit = 0x(ptrval)) [0.00] Call trace: [0.00] show_pte+0x1d0/0x1f0 [0.00] do_mem_abort+0xa8/0xb0 [0.00] el1_da+0x20/0xc4 [0.00] efi_tpm_eventlog_init+0xe8/0x268 [0.00] efi_config_parse_tables+0x180/0x29c [0.00] uefi_init+0x1d0/0x22c [0.00] efi_init+0x90/0x180 [0.00] setup_arch+0x1f4/0x5fc [0.00] start_kernel+0x90/0x51c [0.00] Code: 910d6000 94030b20 17e6 d503201f (d421) [0.00] random: get_random_bytes called from print_oops_end_marker+0x54/0x70 with crng_init=0 [0.00] ---[ end trace ]--- [0.00] Kernel panic - not syncing: Attempted to kill the idle task! [0.00] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
