| Message ID | 00e9b516b13316f7ce81b651957dad0441e56006.1779255851.git.sayalip@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | powerpc/fadump: fix integer overflow in MIN_RMA size check | expand |
Le 20/05/2026 à 07:53, Sayali Patil a écrit : > 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 2048 or larger. This triggers compiler > warnings such as: > > warning: integer overflow in expression of type 'int' > results in '0' [-Woverflow] > > Promote MIN_RMA to u64 before the multiplication so the expression > is evaluated in 64-bit and matches the surrounding physical address > and memory size calculations. > > This fixes both the comparison against ppc64_rma_size and the > assignment to range_start. How do you create that problem ? MIN_RMA has a fixed value of 768 which is defined in asm/prom.h > > Fixes: b7bb46062457 ("powerpc/fadump: fix additional param memory reservation for HASH MMU") > Signed-off-by: Sayali Patil <sayalip@linux.ibm.com> > --- > arch/powerpc/kernel/fadump.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c > index 501d43bf18f3..dea7f7105e42 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 < (u64)MIN_RMA * 1024 * 1024) As you are modifying that line, please use SZ_1M instead of keeping opencoded 1024 * 1024. > return; > > - range_start = MIN_RMA * 1024 * 1024; > + range_start = (u64)MIN_RMA * 1024 * 1024; Same. > range_end = min(ppc64_rma_size, fw_dump.boot_mem_top); > } > Maybe the best would be to define MIN_RMA as a bytes value in asm/prom.h and to divide the value by SZ_1M in kernel/prom_init.c when initialising ibm_architecture_vec_template. That way you could just define it as SZ_2G instead of the problematic 2048 you mention.
On 20/05/26 14:30, Christophe Leroy (CS GROUP) wrote: > > > Le 20/05/2026 à 07:53, Sayali Patil a écrit : >> 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 2048 or larger. This triggers compiler >> warnings such as: >> >> warning: integer overflow in expression of type 'int' >> results in '0' [-Woverflow] >> >> Promote MIN_RMA to u64 before the multiplication so the expression >> is evaluated in 64-bit and matches the surrounding physical address >> and memory size calculations. >> >> This fixes both the comparison against ppc64_rma_size and the >> assignment to range_start. > > How do you create that problem ? MIN_RMA has a fixed value of 768 which > is defined in asm/prom.h Agreed, MIN_RMA is currently fixed at 768MB. While going through the code, I thought this could become a problem if MIN_RMA is increased beyond values such as SZ_2G. > >> >> Fixes: b7bb46062457 ("powerpc/fadump: fix additional param memory >> reservation for HASH MMU") >> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com> >> --- >> arch/powerpc/kernel/fadump.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c >> index 501d43bf18f3..dea7f7105e42 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 < (u64)MIN_RMA * 1024 * 1024) > > As you are modifying that line, please use SZ_1M instead of keeping > opencoded 1024 * 1024. > >> return; >> - range_start = MIN_RMA * 1024 * 1024; >> + range_start = (u64)MIN_RMA * 1024 * 1024; > > Same. > >> range_end = min(ppc64_rma_size, fw_dump.boot_mem_top); >> } > > Maybe the best would be to define MIN_RMA as a bytes value in asm/prom.h > and to divide the value by SZ_1M in kernel/prom_init.c when initialising > ibm_architecture_vec_template. > > That way you could just define it as SZ_2G instead of the problematic > 2048 you mention. > > Thanks for the review Christophe! I will incorporate all the suggested improvements in v2. Regards, Sayali
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 501d43bf18f3..dea7f7105e42 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 < (u64)MIN_RMA * 1024 * 1024) return; - range_start = MIN_RMA * 1024 * 1024; + range_start = (u64)MIN_RMA * 1024 * 1024; range_end = min(ppc64_rma_size, fw_dump.boot_mem_top); }
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 2048 or larger. This triggers compiler warnings such as: warning: integer overflow in expression of type 'int' results in '0' [-Woverflow] Promote MIN_RMA to u64 before the multiplication so the expression is evaluated in 64-bit and matches the surrounding physical address and memory size calculations. This fixes both the comparison against ppc64_rma_size and the assignment to range_start. Fixes: b7bb46062457 ("powerpc/fadump: fix additional param memory reservation for HASH MMU") Signed-off-by: Sayali Patil <sayalip@linux.ibm.com> --- arch/powerpc/kernel/fadump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)