Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1247556 > unrolled thread

[PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov <bp@alien8.de> - 2015-10-15 10:30 +0200
    Re: [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Dave Young <dyoung@redhat.com> - 2015-10-15 11:30 +0200
      Re: [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval Borislav Petkov <bp@alien8.de> - 2015-10-15 12:10 +0200

#1247556 — [PATCH 4/5] x86/setup/crash: Check memblock_reserve() retval

From: Borislav Petkov <bp@suse.de>

memblock_reserve() can fail but the crashkernel reservation code
doesn't check that and this can lead the user into believing that the
crashkernel region was actually reserved. Make sure we check that return
value and we exit early with a failure message in the error case.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jerry_hoemann@hp.com
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: WANG Chao <chaowang@redhat.com>
Cc: x86-ml <x86@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d4788719a1e2..3f75297d5fd0 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -531,7 +531,11 @@ static int __init reserve_crashkernel_low(void)
 		return -ENOMEM;
 	}
 
-	memblock_reserve(low_base, low_size);
+	ret = memblock_reserve(low_base, low_size);
+	if (ret) {
+		pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
+		return ret;
+	}
 
 	pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
 		(unsigned long)(low_size >> 20),
@@ -589,7 +593,11 @@ static void __init reserve_crashkernel(void)
 			return;
 		}
 	}
-	memblock_reserve(crash_base, crash_size);
+	ret = memblock_reserve(crash_base, crash_size);
+	if (ret) {
+		pr_err("%s: Error reserving crashkernel memblock.\n", __func__);
+		return;
+	}
 
 	if (crash_base >= (1ULL << 32) && reserve_crashkernel_low()) {
 		memblock_free(crash_base, crash_size);
-- 
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247609

On 10/15/15 at 10:20am, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> memblock_reserve() can fail but the crashkernel reservation code
> doesn't check that and this can lead the user into believing that the
> crashkernel region was actually reserved. Make sure we check that return
> value and we exit early with a failure message in the error case.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: jerry_hoemann@hp.com
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: WANG Chao <chaowang@redhat.com>
> Cc: x86-ml <x86@kernel.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/kernel/setup.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d4788719a1e2..3f75297d5fd0 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -531,7 +531,11 @@ static int __init reserve_crashkernel_low(void)
>  		return -ENOMEM;
>  	}
>  
> -	memblock_reserve(low_base, low_size);
> +	ret = memblock_reserve(low_base, low_size);
> +	if (ret) {
> +		pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
> +		return ret;
> +	}
>  

Seems there's no checking for other callback to memblock_reserve in setup.c
Need another cleanup?

BTW, a further cleanup is reasonable to me, there's a lot of below patter:
memblock_find_in_range
error checking
memblock_reserve
error checking

So a new function memblock_reserve_in_range is reasonable.

Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247634

On Thu, Oct 15, 2015 at 05:18:26PM +0800, Dave Young wrote:
> Seems there's no checking for other callback to memblock_reserve in setup.c
> Need another cleanup?

True story. It sure does.

> BTW, a further cleanup is reasonable to me, there's a lot of below patter:
> memblock_find_in_range
> error checking
> memblock_reserve
> error checking
> 
> So a new function memblock_reserve_in_range is reasonable.

Well, in some of the callsites, the first "error checking" issues
a specific message, depending on the subsystem. The following
memblock_reserve() is mostly unchecked though. At least that should be
fixed...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web