linux-kernel - Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <77BD029B-35BF-4033-8CCD-DFD0752237D6@zytor.com>
Date: Fri, 16 May 2025 11:17:35 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org
CC: Andy Shevchenko <andy@...nel.org>, Arnd Bergmann <arnd@...nel.org>,
        Borislav Petkov <bp@...en8.de>, Juergen Gross <jgross@...e.com>,
        Kees Cook <keescook@...omium.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mike Rapoport <rppt@...nel.org>, Paul Menzel <pmenzel@...gen.mpg.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        David Woodhouse <dwmw@...zon.co.uk>
Subject: Re: [PATCH 32/32] x86/boot/e820: Move index increments outside accessors in e820__update_table()

On May 15, 2025 5:05:48 AM PDT, Ingo Molnar <mingo@...nel.org> wrote:
>This kind of code:
>
>	change_point[chg_idx++]->entry  = &entries[idx];
>
>Can be a bit confusing to human readers, and GCC-15 started
>warning about these patterns.
>
>Move the index increment outside the accessor.
>
>Suggested-by: Andy Shevchenko <andy@...nel.org>
>Signed-off-by: Ingo Molnar <mingo@...nel.org>
>Cc: Arnd Bergmann <arnd@...nel.org>
>Cc: David Woodhouse <dwmw@...zon.co.uk>
>Cc: H. Peter Anvin <hpa@...or.com>
>Cc: Kees Cook <keescook@...omium.org>
>Cc: Linus Torvalds <torvalds@...ux-foundation.org>
>Cc: Mike Rapoport (Microsoft) <rppt@...nel.org>
>---
> arch/x86/kernel/e820.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
>index 10c6e7dc72d7..afb312620c82 100644
>--- a/arch/x86/kernel/e820.c
>+++ b/arch/x86/kernel/e820.c
>@@ -421,9 +421,11 @@ __init int e820__update_table(struct e820_table *table)
> 	for (idx = 0; idx < table->nr_entries; idx++)	{
> 		if (entries[idx].size != 0) {
> 			change_point[chg_idx]->addr	= entries[idx].addr;
>-			change_point[chg_idx++]->entry	= &entries[idx];
>+			change_point[chg_idx]->entry	= &entries[idx];
>+			chg_idx++;
> 			change_point[chg_idx]->addr	= entries[idx].addr + entries[idx].size;
>-			change_point[chg_idx++]->entry	= &entries[idx];
>+			change_point[chg_idx]->entry	= &entries[idx];
>+			chg_idx++;
> 		}
> 	}
> 	chg_nr = chg_idx;

Really? That seems easier to miss to me.