[PATCH v2] powerpc/fadump: define MIN_RMA in bytes value to fix overflow

[PATCH v2] powerpc/fadump: define MIN_RMA in bytes value to fix overflow

[Sayali Patil]

The MIN_RMA size checks in fadump_setup_param_area() use
(MIN_RMA * 1024 * 1024), which is evaluated in int and can
overflow when MIN_RMA is increased to values such as SZ_2G.
This triggers compiler warnings such as:

warning: integer overflow in expression of type 'int'
results in '0' [-Woverflow]

Define MIN_RMA directly in bytes using SZ_1M and update the
callers accordingly. This avoids repeated unit conversions and
prevents integer overflow.

Also convert MIN_RMA back to MB when populating the firmware
architecture vector, since firmware expects the value in MB.

Fixes: b7bb46062457 ("powerpc/fadump: fix additional param memory reservation for HASH MMU")
Suggested-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
---
 arch/powerpc/include/asm/prom.h | 4 +++-
 arch/powerpc/kernel/fadump.c    | 4 ++--
 arch/powerpc/kernel/prom_init.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index f679a11a7e7f..f4991d10d89e 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -12,12 +12,14 @@
  * Updates for PPC64 by Peter Bergner & David Engebretsen, IBM Corp.
  */
 #include <linux/types.h>
+#include <linux/sizes.h>
 #include <asm/firmware.h>
 
 struct device_node;
 struct property;
 
-#define MIN_RMA			768		/* Minimum RMA (in MB) for CAS negotiation */
+/* Minimum RMA in bytes for CAS negotiation */
+#define MIN_RMA			(768ULL * SZ_1M)
 
 #define OF_DT_BEGIN_NODE	0x1		/* Start of node, full name */
 #define OF_DT_END_NODE		0x2		/* End node */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 501d43bf18f3..bb41150b363c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1759,10 +1759,10 @@ void __init fadump_setup_param_area(void)
 		 * 2. The range should be between MIN_RMA and RMA size (ppc64_rma_size)
 		 * 3. It must not overlap with the fadump reserved area.
 		 */
-		if (ppc64_rma_size < MIN_RMA*1024*1024)
+		if (ppc64_rma_size < MIN_RMA)
 			return;
 
-		range_start = MIN_RMA * 1024 * 1024;
+		range_start = MIN_RMA;
 		range_end = min(ppc64_rma_size, fw_dump.boot_mem_top);
 	}
 
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f26e80cbc615..53503937de0e 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1061,7 +1061,7 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
 		.virt_base = cpu_to_be32(0xffffffff),
 		.virt_size = cpu_to_be32(0xffffffff),
 		.load_base = cpu_to_be32(0xffffffff),
-		.min_rma = cpu_to_be32(MIN_RMA),
+		.min_rma = cpu_to_be32(MIN_RMA / SZ_1M),
 		.min_load = cpu_to_be32(0xffffffff),	/* full client load */
 		.min_rma_percent = 0,	/* min RMA percentage of total RAM */
 		.max_pft_size = 48,	/* max log_2(hash table size) */
-- 
2.52.0

Re: [PATCH v2] powerpc/fadump: define MIN_RMA in bytes value to fix overflow

[Ritesh Harjani]

Sayali Patil <sayalip@linux.ibm.com> writes:

> The MIN_RMA size checks in fadump_setup_param_area() use
> (MIN_RMA * 1024 * 1024), which is evaluated in int and can
> overflow when MIN_RMA is increased to values such as SZ_2G.
> This triggers compiler warnings such as:
>
> warning: integer overflow in expression of type 'int'
> results in '0' [-Woverflow]
>
> Define MIN_RMA directly in bytes using SZ_1M and update the
> callers accordingly. This avoids repeated unit conversions and
> prevents integer overflow.
>
> Also convert MIN_RMA back to MB when populating the firmware
> architecture vector, since firmware expects the value in MB.
>
> Fixes: b7bb46062457 ("powerpc/fadump: fix additional param memory reservation for HASH MMU")

Since this is not a bug per-se, so we need not add a Fixes tag.
But while we are at it, we may correct the commit subject to...

    powerpc/fadump: Define MIN_RMA in bytes rather than MB

... since there is no overflow with it's current value.

With the above changes in place, feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>