Skip to contentSkip to conversation

efi: Fix krealloc defect and misc neatening

PATCH In review Joe Perches 2 participants 8 messages Jan 4, 2014 – Jan 14, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover efi: Fix krealloc defect and misc neatening

Joe Perches (2):
efi: Fix krealloc defect
efi: style neatening

 arch/x86/platform/efi/efi.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)
Signature · 1 line
1.8.1.2.459.gbcd45b4.dirty
#1 lore raw

PATCH 1/2 efi: Fix krealloc defect

krealloc should use a temporary pointer for allocations and check the temporary pointer returned against NULL too.

cc Matthew Garrett <[email protected]>
---
 arch/x86/platform/efi/efi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
arch/x86/platform/efi/efi.c +9 -3
index cceb813..ef7b7b4 100644
@@ -798,6 +798,8 @@ void __init efi_enter_virtual_mode(void)
 	}
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		void *new_memmap2;
+
 		md = p;
 		if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
 #ifdef CONFIG_X86_64
@@ -834,9 +836,13 @@ void __init efi_enter_virtual_mode(void)
 			systab += md->virt_addr - md->phys_addr;
 			efi.systab = (efi_system_table_t *) (unsigned long) systab;
 		}
-		new_memmap = krealloc(new_memmap,
-				      (count + 1) * memmap.desc_size,
-				      GFP_KERNEL);
+		new_memmap2 = krealloc(new_memmap,
+				       (count + 1) * memmap.desc_size,
+				       GFP_KERNEL);
+		if (!new_memmap2)
+			continue;
+
+		new_memmap = new_memmap2;
 		memcpy(new_memmap + (count * memmap.desc_size), md,
 		       memmap.desc_size);
 		count++;
Signature · 1 line
1.8.1.2.459.gbcd45b4.dirty
#2 lore raw

Re: [PATCH 1/2] efi: Fix krealloc defect

Quoted 8 lines · Joe Perches

krealloc should use a temporary pointer for allocations and check the temporary pointer returned against NULL too.

cc Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>
---
 arch/x86/platform/efi/efi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Thanks Joe, but I've already got the following commit queued up from Borislav that addresses this,

---

Quoted 1 line

From ee41143027706d9f342dfe05487a00b20887fde7 Mon Sep 17 00:00:00 2001

From: Borislav Petkov <[email protected]>
Date: Thu, 31 Oct 2013 17:25:09 +0100
Subject: [PATCH] x86/efi: Check krealloc return value

Check it just in case. We might just as well panic there because runtime won't be functioning anyway.

---
 arch/x86/platform/efi/efi.c | 8 ++++++++
 1 file changed, 8 insertions(+)
arch/x86/platform/efi/efi.c +8 -0
index b453069236fd..3fac4dee492f 100644
@@ -860,6 +860,9 @@ void __init efi_enter_virtual_mode(void)
 		new_memmap = krealloc(new_memmap,
 				      (count + 1) * memmap.desc_size,
 				      GFP_KERNEL);
+		if (!new_memmap)
+			goto err_out;
+
 		memcpy(new_memmap + (count * memmap.desc_size), md,
 		       memmap.desc_size);
 		count++;
@@ -914,6 +917,11 @@ void __init efi_enter_virtual_mode(void)
 			 EFI_VARIABLE_BOOTSERVICE_ACCESS |
 			 EFI_VARIABLE_RUNTIME_ACCESS,
 			 0, NULL);
+
+	return;
+
+ err_out:
+	pr_err("Error reallocating memory, EFI runtime non-functional!\n");
 }
 
 /*
Signature · 4 lines
1.8.3.1

-- 
Matt Fleming, Intel Open Source Technology Center
#3 lore raw

Re: [PATCH 1/2] efi: Fix krealloc defect

Quoted 4 lines · Matt Fleming
Quoted 3 lines · Joe Perches

krealloc should use a temporary pointer for allocations and check the temporary pointer returned against NULL too.

Quoted 8 lines
Quoted 5 lines
cc Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>
---
 arch/x86/platform/efi/efi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Thanks Joe, but I've already got the following commit queued up from Borislav that addresses this,

Hi Matt. That's great.

If it's queued up in some tree of yours, should that tree be included in Stephen Rothwell's -next?

cheers, Joe

#4 lore raw

Re: [PATCH 1/2] efi: Fix krealloc defect

Quoted 3 lines · Joe Perches

If it's queued up in some tree of yours, should that tree be included in Stephen Rothwell's -next?

My trees get pulled into tip and those are included in linux-next. So, Borislav's patch is commit ee4114302770 ("x86/efi: Check krealloc return value") in today's linux-next.

Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
#5 lore raw

Re: [PATCH 1/2] efi: Fix krealloc defect

Quoted 6 lines · Matt Fleming
Quoted 2 lines · Joe Perches

If it's queued up in some tree of yours, should that tree be included in Stephen Rothwell's -next?

My trees get pulled into tip and those are included in linux-next. So, Borislav's patch is commit ee4114302770 ("x86/efi: Check krealloc return value") in today's linux-next.

You're right, thanks.
I thought I did this against -next but it appears not.

cheers, Joe

#6 lore raw

PATCH 2/2 efi: style neatening

In reply to Joe Perches

Coalesce formats and remove spaces before tabs.
Move __initdata after the variable declaration.

---
 arch/x86/platform/efi/efi.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)
arch/x86/platform/efi/efi.c +13 -17
index ef7b7b4..69c6133 100644
@@ -67,7 +67,7 @@ static efi_system_table_t efi_systab __initdata;
 
 unsigned long x86_efi_facility;
 
-static __initdata efi_config_table_type_t arch_tables[] = {
+static efi_config_table_type_t arch_tables[] __initdata = {
 #ifdef CONFIG_X86_UV
 	{UV_SYSTEM_TABLE_GUID, "UVsystab", &efi.uv_systab},
 #endif
@@ -83,7 +83,7 @@ int efi_enabled(int facility)
 }
 EXPORT_SYMBOL(efi_enabled);
 
-static bool __initdata disable_runtime = false;
+static bool disable_runtime __initdata = false;
 static int __init setup_noefi(char *arg)
 {
 	disable_runtime = true;
@@ -271,9 +271,9 @@ static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
 int efi_set_rtc_mmss(const struct timespec *now)
 {
 	unsigned long nowtime = now->tv_sec;
-	efi_status_t 	status;
-	efi_time_t 	eft;
-	efi_time_cap_t 	cap;
+	efi_status_t	status;
+	efi_time_t	eft;
+	efi_time_cap_t	cap;
 	struct rtc_time	tm;
 
 	status = efi.get_time(&eft, &cap);
@@ -291,9 +291,8 @@ int efi_set_rtc_mmss(const struct timespec *now)
 		eft.second = tm.tm_sec;
 		eft.nanosecond = 0;
 	} else {
-		printk(KERN_ERR
-		       "%s: Invalid EFI RTC value: write of %lx to EFI RTC failed\n",
-		       __FUNCTION__, nowtime);
+		pr_err("%s: Invalid EFI RTC value: write of %lx to EFI RTC failed\n",
+		       __func__, nowtime);
 		return -1;
 	}
 
@@ -409,8 +408,7 @@ static void __init print_efi_memmap(void)
 	     p < memmap.map_end;
 	     p += memmap.desc_size, i++) {
 		md = p;
-		pr_info("mem%02u: type=%u, attr=0x%llx, "
-			"range=[0x%016llx-0x%016llx) (%lluMB)\n",
+		pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
 			i, md->type, md->attribute, md->phys_addr,
 			md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
 			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
@@ -442,9 +440,8 @@ void __init efi_reserve_boot_services(void)
 			memblock_is_region_reserved(start, size)) {
 			/* Could not reserve, skip it */
 			md->num_pages = 0;
-			memblock_dbg("Could not reserve boot range "
-					"[0x%010llx-0x%010llx]\n",
-						start, start+size-1);
+			memblock_dbg("Could not reserve boot range [0x%010llx-0x%010llx]\n",
+				     start, start+size-1);
 		} else
 			memblock_reserve(start, size);
 	}
@@ -566,8 +563,7 @@ static int __init efi_systab_init(void *phys)
 		return -EINVAL;
 	}
 	if ((efi.systab->hdr.revision >> 16) == 0)
-		pr_err("Warning: System table version "
-		       "%d.%02d, expected 1.00 or greater!\n",
+		pr_err("Warning: System table version %d.%02d, expected 1.00 or greater!\n",
 		       efi.systab->hdr.revision >> 16,
 		       efi.systab->hdr.revision & 0xffff);
 
@@ -857,8 +853,8 @@ void __init efi_enter_virtual_mode(void)
 		(efi_memory_desc_t *)__pa(new_memmap));
 
 	if (status != EFI_SUCCESS) {
-		pr_alert("Unable to switch EFI into virtual mode "
-			 "(status=%lx)!\n", status);
+		pr_alert("Unable to switch EFI into virtual mode (status=%lx)!\n",
+			 status);
 		panic("EFI call to SetVirtualAddressMap() failed!");
 	}
 
Signature · 1 line
1.8.1.2.459.gbcd45b4.dirty
#7 lore raw

Re: [PATCH 2/2] efi: style neatening

Quoted 7 lines · Joe Perches

Coalesce formats and remove spaces before tabs.
Move __initdata after the variable declaration.

---
 arch/x86/platform/efi/efi.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

Thanks Joe, I picked this one up.

Signature · 1 line
Matt Fleming, Intel Open Source Technology Center