* [PATCH] efi/libstub/arm64: Report meaningful relocation errors
@ 2019-08-14 20:55 Kees Cook
2019-09-04 10:38 ` Will Deacon
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-08-14 20:55 UTC (permalink / raw)
To: Will Deacon; +Cc: Ard Biesheuvel, linux-kernel, Mark Rutland
When UEFI booting, if allocate_pages() fails (either via KASLR or
regular boot), efi_low_alloc() is used for fall back. If it, too, fails,
it reports "Failed to relocate kernel". Then handle_kernel_image()
reports the failure to its caller, which unhelpfully reports exactly
the same string again:
EFI stub: ERROR: Failed to relocate kernel
EFI stub: ERROR: Failed to relocate kernel
While debugging linker errors in the UEFI code that created insane memory
sizes that all the allocation attempts would fail at, this was a cause
for confusion. Knowing each allocation had failed would have helped me
isolate the issue sooner. To that end, this improves the error messages
to detail which specific allocations have failed.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/firmware/efi/libstub/arm64-stub.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 1550d244e996..24022f956e01 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
status = efi_random_alloc(sys_table_arg, *reserve_size,
MIN_KIMG_ALIGN, reserve_addr,
(u32)phys_seed);
+ if (status != EFI_SUCCESS)
+ pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
*image_addr = *reserve_addr + offset;
} else {
@@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
EFI_LOADER_DATA,
*reserve_size / EFI_PAGE_SIZE,
(efi_physical_addr_t *)reserve_addr);
+ if (status != EFI_SUCCESS)
+ pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
}
if (status != EFI_SUCCESS) {
@@ -143,7 +147,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
MIN_KIMG_ALIGN, reserve_addr);
if (status != EFI_SUCCESS) {
- pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
+ pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
*reserve_size = 0;
return status;
}
--
2.17.1
--
Kees Cook
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-08-14 20:55 [PATCH] efi/libstub/arm64: Report meaningful relocation errors Kees Cook
@ 2019-09-04 10:38 ` Will Deacon
2019-09-04 20:38 ` Kees Cook
0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2019-09-04 10:38 UTC (permalink / raw)
To: Kees Cook; +Cc: Ard Biesheuvel, linux-kernel, Mark Rutland
Hi Kees,
On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> When UEFI booting, if allocate_pages() fails (either via KASLR or
> regular boot), efi_low_alloc() is used for fall back. If it, too, fails,
> it reports "Failed to relocate kernel". Then handle_kernel_image()
> reports the failure to its caller, which unhelpfully reports exactly
> the same string again:
>
> EFI stub: ERROR: Failed to relocate kernel
> EFI stub: ERROR: Failed to relocate kernel
>
> While debugging linker errors in the UEFI code that created insane memory
> sizes that all the allocation attempts would fail at, this was a cause
> for confusion. Knowing each allocation had failed would have helped me
> isolate the issue sooner. To that end, this improves the error messages
> to detail which specific allocations have failed.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/firmware/efi/libstub/arm64-stub.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 1550d244e996..24022f956e01 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> status = efi_random_alloc(sys_table_arg, *reserve_size,
> MIN_KIMG_ALIGN, reserve_addr,
> (u32)phys_seed);
> + if (status != EFI_SUCCESS)
> + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
>
> *image_addr = *reserve_addr + offset;
> } else {
> @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> EFI_LOADER_DATA,
> *reserve_size / EFI_PAGE_SIZE,
> (efi_physical_addr_t *)reserve_addr);
> + if (status != EFI_SUCCESS)
> + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> }
Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
case -- only one should run, right? That also didn't seem to be part of
the use-case in the commit, unless I'm missing something.
Maybe combine the prints as per the diff below?
Will
--->8
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 1550d244e996..820c58cc149e 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -143,13 +143,15 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
MIN_KIMG_ALIGN, reserve_addr);
if (status != EFI_SUCCESS) {
- pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
+ pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
*reserve_size = 0;
return status;
}
*image_addr = *reserve_addr + TEXT_OFFSET;
+ } else {
+ pr_efi_err(sys_table_arg, "allocate_pages() failed\n");
}
- memcpy((void *)*image_addr, old_image_addr, kernel_size);
+ memcpy((void *)*image_addr, old_image_addr, kernel_size);
return EFI_SUCCESS;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-09-04 10:38 ` Will Deacon
@ 2019-09-04 20:38 ` Kees Cook
2019-09-06 10:44 ` Will Deacon
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-04 20:38 UTC (permalink / raw)
To: Will Deacon; +Cc: Ard Biesheuvel, linux-kernel, Mark Rutland
On Wed, Sep 04, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> Hi Kees,
>
> On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> > When UEFI booting, if allocate_pages() fails (either via KASLR or
> > regular boot), efi_low_alloc() is used for fall back. If it, too, fails,
> > it reports "Failed to relocate kernel". Then handle_kernel_image()
> > reports the failure to its caller, which unhelpfully reports exactly
> > the same string again:
> >
> > EFI stub: ERROR: Failed to relocate kernel
> > EFI stub: ERROR: Failed to relocate kernel
> >
> > While debugging linker errors in the UEFI code that created insane memory
> > sizes that all the allocation attempts would fail at, this was a cause
> > for confusion. Knowing each allocation had failed would have helped me
> > isolate the issue sooner. To that end, this improves the error messages
> > to detail which specific allocations have failed.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > drivers/firmware/efi/libstub/arm64-stub.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > index 1550d244e996..24022f956e01 100644
> > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > status = efi_random_alloc(sys_table_arg, *reserve_size,
> > MIN_KIMG_ALIGN, reserve_addr,
> > (u32)phys_seed);
> > + if (status != EFI_SUCCESS)
> > + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
> >
> > *image_addr = *reserve_addr + offset;
> > } else {
> > @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > EFI_LOADER_DATA,
> > *reserve_size / EFI_PAGE_SIZE,
> > (efi_physical_addr_t *)reserve_addr);
> > + if (status != EFI_SUCCESS)
> > + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> > }
>
> Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
> case -- only one should run, right? That also didn't seem to be part of
> the use-case in the commit, unless I'm missing something.
I just did that to help with differentiating the cases. Maybe something
was special about KASLR picking the wrong location that triggered the
failure, etc.
> Maybe combine the prints as per the diff below?
That could work. If you're against the KASLR vs regular thing, I can
respin the patch?
-Kees
>
> Will
>
> --->8
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 1550d244e996..820c58cc149e 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -143,13 +143,15 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> MIN_KIMG_ALIGN, reserve_addr);
>
> if (status != EFI_SUCCESS) {
> - pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
> + pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
> *reserve_size = 0;
> return status;
> }
> *image_addr = *reserve_addr + TEXT_OFFSET;
> + } else {
> + pr_efi_err(sys_table_arg, "allocate_pages() failed\n");
> }
> - memcpy((void *)*image_addr, old_image_addr, kernel_size);
>
> + memcpy((void *)*image_addr, old_image_addr, kernel_size);
> return EFI_SUCCESS;
> }
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-09-04 20:38 ` Kees Cook
@ 2019-09-06 10:44 ` Will Deacon
2019-09-06 17:34 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2019-09-06 10:44 UTC (permalink / raw)
To: Kees Cook; +Cc: Ard Biesheuvel, linux-kernel, Mark Rutland
On Wed, Sep 04, 2019 at 01:38:04PM -0700, Kees Cook wrote:
> On Wed, Sep 04, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > > index 1550d244e996..24022f956e01 100644
> > > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > > @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > status = efi_random_alloc(sys_table_arg, *reserve_size,
> > > MIN_KIMG_ALIGN, reserve_addr,
> > > (u32)phys_seed);
> > > + if (status != EFI_SUCCESS)
> > > + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
> > >
> > > *image_addr = *reserve_addr + offset;
> > > } else {
> > > @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > EFI_LOADER_DATA,
> > > *reserve_size / EFI_PAGE_SIZE,
> > > (efi_physical_addr_t *)reserve_addr);
> > > + if (status != EFI_SUCCESS)
> > > + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> > > }
> >
> > Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
> > case -- only one should run, right? That also didn't seem to be part of
> > the use-case in the commit, unless I'm missing something.
>
> I just did that to help with differentiating the cases. Maybe something
> was special about KASLR picking the wrong location that triggered the
> failure, etc.
>
> > Maybe combine the prints as per the diff below?
>
> That could work. If you're against the KASLR vs regular thing, I can
> respin the patch?
Happy to Ack it with that change, although I suppose it's ultimately up
to Ard :)
Will
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-09-06 10:44 ` Will Deacon
@ 2019-09-06 17:34 ` Ard Biesheuvel
2019-09-23 23:17 ` Kees Cook
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2019-09-06 17:34 UTC (permalink / raw)
To: Will Deacon; +Cc: Kees Cook, Linux Kernel Mailing List, Mark Rutland
On Fri, 6 Sep 2019 at 03:44, Will Deacon <will@kernel.org> wrote:
>
> On Wed, Sep 04, 2019 at 01:38:04PM -0700, Kees Cook wrote:
> > On Wed, Sep 04, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > > On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> > > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > index 1550d244e996..24022f956e01 100644
> > > > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > status = efi_random_alloc(sys_table_arg, *reserve_size,
> > > > MIN_KIMG_ALIGN, reserve_addr,
> > > > (u32)phys_seed);
> > > > + if (status != EFI_SUCCESS)
> > > > + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
> > > >
> > > > *image_addr = *reserve_addr + offset;
> > > > } else {
> > > > @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > EFI_LOADER_DATA,
> > > > *reserve_size / EFI_PAGE_SIZE,
> > > > (efi_physical_addr_t *)reserve_addr);
> > > > + if (status != EFI_SUCCESS)
> > > > + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> > > > }
> > >
> > > Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
> > > case -- only one should run, right? That also didn't seem to be part of
> > > the use-case in the commit, unless I'm missing something.
> >
> > I just did that to help with differentiating the cases. Maybe something
> > was special about KASLR picking the wrong location that triggered the
> > failure, etc.
> >
> > > Maybe combine the prints as per the diff below?
> >
> > That could work. If you're against the KASLR vs regular thing, I can
> > respin the patch?
>
> Happy to Ack it with that change, although I suppose it's ultimately up
> to Ard :)
>
No objections from me, but I prefer Will's version.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-09-06 17:34 ` Ard Biesheuvel
@ 2019-09-23 23:17 ` Kees Cook
2019-09-25 15:38 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-23 23:17 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Will Deacon, Linux Kernel Mailing List, Mark Rutland
On Fri, Sep 06, 2019 at 10:34:47AM -0700, Ard Biesheuvel wrote:
> On Fri, 6 Sep 2019 at 03:44, Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Sep 04, 2019 at 01:38:04PM -0700, Kees Cook wrote:
> > > On Wed, Sep 04, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > > > On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> > > > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > index 1550d244e996..24022f956e01 100644
> > > > > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > > status = efi_random_alloc(sys_table_arg, *reserve_size,
> > > > > MIN_KIMG_ALIGN, reserve_addr,
> > > > > (u32)phys_seed);
> > > > > + if (status != EFI_SUCCESS)
> > > > > + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
> > > > >
> > > > > *image_addr = *reserve_addr + offset;
> > > > > } else {
> > > > > @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > > EFI_LOADER_DATA,
> > > > > *reserve_size / EFI_PAGE_SIZE,
> > > > > (efi_physical_addr_t *)reserve_addr);
> > > > > + if (status != EFI_SUCCESS)
> > > > > + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> > > > > }
> > > >
> > > > Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
> > > > case -- only one should run, right? That also didn't seem to be part of
> > > > the use-case in the commit, unless I'm missing something.
> > >
> > > I just did that to help with differentiating the cases. Maybe something
> > > was special about KASLR picking the wrong location that triggered the
> > > failure, etc.
> > >
> > > > Maybe combine the prints as per the diff below?
> > >
> > > That could work. If you're against the KASLR vs regular thing, I can
> > > respin the patch?
> >
> > Happy to Ack it with that change, although I suppose it's ultimately up
> > to Ard :)
> >
>
> No objections from me, but I prefer Will's version.
I took a look at this again... to report the failures as Will suggests,
it would look like this:
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -138,12 +138,14 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
}
if (status != EFI_SUCCESS) {
+ pr_efi_err(sys_table_arg, "allocate_pages() failed\n");
+
*reserve_size = kernel_memsize + TEXT_OFFSET;
status = efi_low_alloc(sys_table_arg, *reserve_size,
MIN_KIMG_ALIGN, reserve_addr);
if (status != EFI_SUCCESS) {
- pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
+ pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
*reserve_size = 0;
return status;
}
My reasoning for putting the failure earlier is to differentiate which
path was taken where the allocate_pages() failed: either regular or
KASLR. If that's really not considered important here, I can send the
above patch... Thoughts?
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] efi/libstub/arm64: Report meaningful relocation errors
2019-09-23 23:17 ` Kees Cook
@ 2019-09-25 15:38 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2019-09-25 15:38 UTC (permalink / raw)
To: Kees Cook; +Cc: Will Deacon, Linux Kernel Mailing List, Mark Rutland
On Tue, 24 Sep 2019 at 01:17, Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Sep 06, 2019 at 10:34:47AM -0700, Ard Biesheuvel wrote:
> > On Fri, 6 Sep 2019 at 03:44, Will Deacon <will@kernel.org> wrote:
> > >
> > > On Wed, Sep 04, 2019 at 01:38:04PM -0700, Kees Cook wrote:
> > > > On Wed, Sep 04, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > > > > On Wed, Aug 14, 2019 at 01:55:50PM -0700, Kees Cook wrote:
> > > > > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > > index 1550d244e996..24022f956e01 100644
> > > > > > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > > > > > @@ -111,6 +111,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > > > status = efi_random_alloc(sys_table_arg, *reserve_size,
> > > > > > MIN_KIMG_ALIGN, reserve_addr,
> > > > > > (u32)phys_seed);
> > > > > > + if (status != EFI_SUCCESS)
> > > > > > + pr_efi_err(sys_table_arg, "KASLR allocate_pages() failed\n");
> > > > > >
> > > > > > *image_addr = *reserve_addr + offset;
> > > > > > } else {
> > > > > > @@ -135,6 +137,8 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> > > > > > EFI_LOADER_DATA,
> > > > > > *reserve_size / EFI_PAGE_SIZE,
> > > > > > (efi_physical_addr_t *)reserve_addr);
> > > > > > + if (status != EFI_SUCCESS)
> > > > > > + pr_efi_err(sys_table_arg, "regular allocate_pages() failed\n");
> > > > > > }
> > > > >
> > > > > Not sure I see the need to distinsuish the 'KASLR' case from the 'regular'
> > > > > case -- only one should run, right? That also didn't seem to be part of
> > > > > the use-case in the commit, unless I'm missing something.
> > > >
> > > > I just did that to help with differentiating the cases. Maybe something
> > > > was special about KASLR picking the wrong location that triggered the
> > > > failure, etc.
> > > >
> > > > > Maybe combine the prints as per the diff below?
> > > >
> > > > That could work. If you're against the KASLR vs regular thing, I can
> > > > respin the patch?
> > >
> > > Happy to Ack it with that change, although I suppose it's ultimately up
> > > to Ard :)
> > >
> >
> > No objections from me, but I prefer Will's version.
>
> I took a look at this again... to report the failures as Will suggests,
> it would look like this:
>
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -138,12 +138,14 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> }
>
> if (status != EFI_SUCCESS) {
> + pr_efi_err(sys_table_arg, "allocate_pages() failed\n");
> +
> *reserve_size = kernel_memsize + TEXT_OFFSET;
> status = efi_low_alloc(sys_table_arg, *reserve_size,
> MIN_KIMG_ALIGN, reserve_addr);
>
> if (status != EFI_SUCCESS) {
> - pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
> + pr_efi_err(sys_table_arg, "efi_low_alloc() failed\n");
> *reserve_size = 0;
> return status;
> }
>
> My reasoning for putting the failure earlier is to differentiate which
> path was taken where the allocate_pages() failed: either regular or
> KASLR. If that's really not considered important here, I can send the
> above patch... Thoughts?
>
The first pr_efi_err() in the patch above complains about a condition
that is not actually an error.
If you are interested in recording the path taken through this
function, I have no objections to putting a normal pr_efi() print
inside the KASLR block that shows that the physical placement of the
kernel is being randomized. Then, we can keep only the second
pr_efi_err() above to report the failure.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-25 15:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 20:55 [PATCH] efi/libstub/arm64: Report meaningful relocation errors Kees Cook
2019-09-04 10:38 ` Will Deacon
2019-09-04 20:38 ` Kees Cook
2019-09-06 10:44 ` Will Deacon
2019-09-06 17:34 ` Ard Biesheuvel
2019-09-23 23:17 ` Kees Cook
2019-09-25 15:38 ` Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.