From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org ([203.11.71.1]:41697 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725951AbeHFKVz (ORCPT ); Mon, 6 Aug 2018 06:21:55 -0400 From: Michael Ellerman To: Hari Bathini , Mahesh Jagannath Salgaonkar , Hari Bathini Cc: stable@vger.kernel.org, linuxppc-dev Subject: Re: [PATCH] powerpc/fadump: handle crash memory ranges array overflow In-Reply-To: References: <153304539025.23724.13483958866671131484.stgit@hbathini.in.ibm.com> Date: Mon, 06 Aug 2018 18:13:56 +1000 Message-ID: <87effboqt7.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: stable-owner@vger.kernel.org List-ID: Hari Bathini writes: > On Monday 06 August 2018 09:52 AM, Mahesh Jagannath Salgaonkar wrote: >> On 07/31/2018 07:26 PM, Hari Bathini wrote: >>> Crash memory ranges is an array of memory ranges of the crashing kernel >>> to be exported as a dump via /proc/vmcore file. The size of the array >>> is set based on INIT_MEMBLOCK_REGIONS, which works alright in most cases >>> where memblock memory regions count is less than INIT_MEMBLOCK_REGIONS >>> value. But this count can grow beyond INIT_MEMBLOCK_REGIONS value since >>> commit 142b45a72e22 ("memblock: Add array resizing support"). >>> ... >>> >>> Fixes: 2df173d9e85d ("fadump: Initialize elfcore header and add PT_LOAD program headers.") >>> Cc: stable@vger.kernel.org >>> Cc: Mahesh Salgaonkar >>> Signed-off-by: Hari Bathini >>> --- >>> arch/powerpc/include/asm/fadump.h | 2 + >>> arch/powerpc/kernel/fadump.c | 63 ++++++++++++++++++++++++++++++++++--- >>> 2 files changed, 59 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h >>> index 5a23010..ff708b3 100644 >>> --- a/arch/powerpc/include/asm/fadump.h >>> +++ b/arch/powerpc/include/asm/fadump.h >>> @@ -196,7 +196,7 @@ struct fadump_crash_info_header { >>> }; >>> >>> /* Crash memory ranges */ >>> -#define INIT_CRASHMEM_RANGES (INIT_MEMBLOCK_REGIONS + 2) >>> +#define INIT_CRASHMEM_RANGES INIT_MEMBLOCK_REGIONS >>> >>> struct fad_crash_memory_ranges { >>> unsigned long long base; >>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c >>> index 07e8396..1c1df4f 100644 >>> --- a/arch/powerpc/kernel/fadump.c >>> +++ b/arch/powerpc/kernel/fadump.c ... > >> Also alongwith this change, Should we also double the initial array size >> (e.g. INIT_CRASHMEM_RANGES * 2) to reduce our chances to go for memory >> allocation ? > > Agreed that doubling the static array size reduces the likelihood of the > need for > dynamic array resizing. Will do that. > > Nonetheless, if we get to the point where 2K memory allocation fails on > a system with so many memory ranges, it is likely that the kernel has some basic > problems to deal with first :) Yes, this all seems a bit silly. Why not just allocate a 64K page and be done with it? AFAICS we're not being called too early to do that, and if you can't allocate a single page then the system is going to OOM anyway. cheers