[PATCH 0/2] efi: Fix krealloc defect and misc neatening

[PATCH 0/2] efi: Fix krealloc defect and misc neatening

[Joe Perches]

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

 arch/x86/platform/efi/efi.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

[PATCH 1/2] efi: Fix krealloc defect

[Joe Perches]

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

Signed-off-by: Joe Perches <joe@perches.com>
cc: Matthew Garrett <matthew.garrett@nebula.com>
---
 arch/x86/platform/efi/efi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index cceb813..ef7b7b4 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -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++;
-- 
1.8.1.2.459.gbcd45b4.dirty

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

[Matt Fleming]

On Fri, 03 Jan, at 04:08:47PM, Joe Perches wrote:
> krealloc should use a temporary pointer for allocations
> and check the temporary pointer returned against NULL too.
> 
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> cc: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> ---
>  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,

---

>From ee41143027706d9f342dfe05487a00b20887fde7 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
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.

Signed-off-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 arch/x86/platform/efi/efi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index b453069236fd..3fac4dee492f 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -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");
 }
 
 /*
-- 
1.8.3.1

-- 
Matt Fleming, Intel Open Source Technology Center

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

[Joe Perches]

On Mon, 2014-01-13 at 14:03 +0000, Matt Fleming wrote:
> On Fri, 03 Jan, at 04:08:47PM, Joe Perches wrote:
> > krealloc should use a temporary pointer for allocations
> > and check the temporary pointer returned against NULL too.
> > 
> > Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> > cc: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  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

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

[Matt Fleming]

On Mon, 13 Jan, at 09:47:21AM, Joe Perches wrote:
> 
> 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.

-- 
Matt Fleming, Intel Open Source Technology Center

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

[Joe Perches]

On Mon, 2014-01-13 at 19:17 +0000, Matt Fleming wrote:
> On Mon, 13 Jan, at 09:47:21AM, Joe Perches wrote:
> > 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

[PATCH 2/2] efi: style neatening

[Joe Perches]

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

Signed-off-by: Joe Perches <joe@perches.com>
---
 arch/x86/platform/efi/efi.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index ef7b7b4..69c6133 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -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!");
 	}
 
-- 
1.8.1.2.459.gbcd45b4.dirty

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

[Matt Fleming]

On Fri, 03 Jan, at 04:08:48PM, Joe Perches wrote:
> Coalesce formats and remove spaces before tabs.
> Move __initdata after the variable declaration.
> 
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/x86/platform/efi/efi.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)

Thanks Joe, I picked this one up.

-- 
Matt Fleming, Intel Open Source Technology Center