diff mbox

efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

Message ID 1480010543-25709-2-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 018edcfac4c3b140366ad51b0907f3becb5bb624
Headers show

Commit Message

Ard Biesheuvel Nov. 24, 2016, 6:02 p.m. UTC
The UEFI stub executes in the context of the firmware, which identity
maps the available system RAM, which implies that only memory below
4 GB can be used for allocations on 32-bit architectures, even on [L]PAE
capable hardware.

So ignore any reported memory above 4 GB in efi_random_alloc(). This
also fixes a reported build problem on ARM under -Os, where the 64-bit
logical shift relies on a software routine that the ARM decompressor does
not provide.

A second [minor] issue is also fixed, where the '+ 1' is moved out of
the shift, where it belongs: the reason for its presence is that a
memory region where start == end should count as a single slot, given
that 'end' takes the desired size and alignment of the allocation into
account.

To clarify the code in this regard, rename start/end to 'first_slot' and
'last_slot', respectively, and introduce 'region_end' to describe the
last usable address of the current region.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 drivers/firmware/efi/libstub/random.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.7.4

Comments

Matt Fleming Dec. 4, 2016, 2:17 p.m. UTC | #1
On Thu, 24 Nov, at 10:47:15PM, tip-bot for Ard Biesheuvel wrote:
> Commit-ID:  018edcfac4c3b140366ad51b0907f3becb5bb624

> Gitweb:     http://git.kernel.org/tip/018edcfac4c3b140366ad51b0907f3becb5bb624

> Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>

> AuthorDate: Thu, 24 Nov 2016 18:02:23 +0000

> Committer:  Ingo Molnar <mingo@kernel.org>

> CommitDate: Fri, 25 Nov 2016 07:15:23 +0100

> 

> efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

> 

> The UEFI stub executes in the context of the firmware, which identity

> maps the available system RAM, which implies that only memory below

> 4 GB can be used for allocations on 32-bit architectures, even on [L]PAE

> capable hardware.

> 

> So ignore any reported memory above 4 GB in efi_random_alloc(). This

> also fixes a reported build problem on ARM under -Os, where the 64-bit

> logical shift relies on a software routine that the ARM decompressor does

> not provide.

> 

> A second [minor] issue is also fixed, where the '+ 1' is moved out of

> the shift, where it belongs: the reason for its presence is that a

> memory region where start == end should count as a single slot, given

> that 'end' takes the desired size and alignment of the allocation into

> account.

> 

> To clarify the code in this regard, rename start/end to 'first_slot' and

> 'last_slot', respectively, and introduce 'region_end' to describe the

> last usable address of the current region.

> 

> Reported-by: Arnd Bergmann <arnd@arndb.de>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Linus Torvalds <torvalds@linux-foundation.org>

> Cc: Matt Fleming <matt@codeblueprint.co.uk>

> Cc: Peter Zijlstra <peterz@infradead.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>

> Cc: linux-efi@vger.kernel.org

> Link: http://lkml.kernel.org/r/1480010543-25709-2-git-send-email-ard.biesheuvel@linaro.org

> Signed-off-by: Ingo Molnar <mingo@kernel.org>

> ---

>  drivers/firmware/efi/libstub/random.c | 13 +++++++------

>  1 file changed, 7 insertions(+), 6 deletions(-)


Ard, was this picked up for the correct tip branch? If it fixes a
build failure it should have gone into tip/efi/urgent, right?
Ard Biesheuvel Dec. 4, 2016, 2:31 p.m. UTC | #2
On 4 December 2016 at 14:17, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Thu, 24 Nov, at 10:47:15PM, tip-bot for Ard Biesheuvel wrote:

>> Commit-ID:  018edcfac4c3b140366ad51b0907f3becb5bb624

>> Gitweb:     http://git.kernel.org/tip/018edcfac4c3b140366ad51b0907f3becb5bb624

>> Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> AuthorDate: Thu, 24 Nov 2016 18:02:23 +0000

>> Committer:  Ingo Molnar <mingo@kernel.org>

>> CommitDate: Fri, 25 Nov 2016 07:15:23 +0100

>>

>> efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit

>>

>> The UEFI stub executes in the context of the firmware, which identity

>> maps the available system RAM, which implies that only memory below

>> 4 GB can be used for allocations on 32-bit architectures, even on [L]PAE

>> capable hardware.

>>

>> So ignore any reported memory above 4 GB in efi_random_alloc(). This

>> also fixes a reported build problem on ARM under -Os, where the 64-bit

>> logical shift relies on a software routine that the ARM decompressor does

>> not provide.

>>

>> A second [minor] issue is also fixed, where the '+ 1' is moved out of

>> the shift, where it belongs: the reason for its presence is that a

>> memory region where start == end should count as a single slot, given

>> that 'end' takes the desired size and alignment of the allocation into

>> account.

>>

>> To clarify the code in this regard, rename start/end to 'first_slot' and

>> 'last_slot', respectively, and introduce 'region_end' to describe the

>> last usable address of the current region.

>>

>> Reported-by: Arnd Bergmann <arnd@arndb.de>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> Cc: Linus Torvalds <torvalds@linux-foundation.org>

>> Cc: Matt Fleming <matt@codeblueprint.co.uk>

>> Cc: Peter Zijlstra <peterz@infradead.org>

>> Cc: Thomas Gleixner <tglx@linutronix.de>

>> Cc: linux-efi@vger.kernel.org

>> Link: http://lkml.kernel.org/r/1480010543-25709-2-git-send-email-ard.biesheuvel@linaro.org

>> Signed-off-by: Ingo Molnar <mingo@kernel.org>

>> ---

>>  drivers/firmware/efi/libstub/random.c | 13 +++++++------

>>  1 file changed, 7 insertions(+), 6 deletions(-)

>

> Ard, was this picked up for the correct tip branch? If it fixes a

> build failure it should have gone into tip/efi/urgent, right?


The failure was in -next, with a patch queued up for v4.10, so that is
where the fix went as well.
Matt Fleming Dec. 4, 2016, 9:38 p.m. UTC | #3
On Sun, 04 Dec, at 02:31:23PM, Ard Biesheuvel wrote:
> On 4 December 2016 at 14:17, Matt Fleming <matt@codeblueprint.co.uk> wrote:

> >

> > Ard, was this picked up for the correct tip branch? If it fixes a

> > build failure it should have gone into tip/efi/urgent, right?

> 

> The failure was in -next, with a patch queued up for v4.10, so that is

> where the fix went as well.


Oops, sorry I missed that. I was looking at the v4.11 queue and my
brain suffered an off-by-one bug - I thought we'd already had the
v4.10 release.
Ard Biesheuvel Dec. 5, 2016, 9:01 a.m. UTC | #4
On 4 December 2016 at 21:38, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Sun, 04 Dec, at 02:31:23PM, Ard Biesheuvel wrote:

>> On 4 December 2016 at 14:17, Matt Fleming <matt@codeblueprint.co.uk> wrote:

>> >

>> > Ard, was this picked up for the correct tip branch? If it fixes a

>> > build failure it should have gone into tip/efi/urgent, right?

>>

>> The failure was in -next, with a patch queued up for v4.10, so that is

>> where the fix went as well.

>

> Oops, sorry I missed that. I was looking at the v4.11 queue and my

> brain suffered an off-by-one bug - I thought we'd already had the

> v4.10 release.


We haven't even had the v4.9 release :-)
diff mbox

Patch

diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 3a3feacc329f..63cd3f262b6e 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -45,19 +45,21 @@  static unsigned long get_entry_num_slots(efi_memory_desc_t *md,
 					 unsigned long align_shift)
 {
 	unsigned long align = 1UL << align_shift;
-	u64 start, end;
+	u64 first_slot, last_slot, region_end;
 
 	if (md->type != EFI_CONVENTIONAL_MEMORY)
 		return 0;
 
-	start = round_up(md->phys_addr, align);
-	end = round_down(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - size,
-			 align);
+	region_end = min((u64)ULONG_MAX,
+			 md->phys_addr + md->num_pages * EFI_PAGE_SIZE - 1);
 
-	if (start > end)
+	first_slot = round_up(md->phys_addr, align);
+	last_slot = round_down(region_end - size + 1, align);
+
+	if (first_slot > last_slot)
 		return 0;
 
-	return (end - start + 1) >> align_shift;
+	return ((unsigned long)(last_slot - first_slot) >> align_shift) + 1;
 }
 
 /*