linux-kernel - [PATCH v2][next] x86/mm/pgtable: Fix -Wstringop-overflow warnings

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-next>] [day] [month] [year] [list]

Message-ID: <20220509194541.GA91598@embeddedor>
Date:   Mon, 9 May 2022 14:45:41 -0500
From:   "Gustavo A. R. Silva" <gustavoars@...nel.org>
To:     Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        linux-hardening@...r.kernel.org
Subject: [PATCH v2][next] x86/mm/pgtable: Fix -Wstringop-overflow warnings

Fix the following -Wstringop-overflow warnings when building with GCC-12.1:

arch/x86/mm/pgtable.c:437:13: warning: 'preallocate_pmds' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
arch/x86/mm/pgtable.c:440:13: warning: 'preallocate_pmds' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
arch/x86/mm/pgtable.c:462:9: warning: 'free_pmds' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
arch/x86/mm/pgtable.c:454:9: warning: 'pgd_prepopulate_pmd' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
arch/x86/mm/pgtable.c:455:9: warning: 'pgd_prepopulate_user_pmd' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]
arch/x86/mm/pgtable.c:464:9: warning: 'free_pmds' accessing 8 bytes in a region of size 0 [-Wstringop-overflow=]

There is a case in which PREALLOCATED_PMDS, MAX_PREALLOCATED_PMDS,
PREALLOCATED_USER_PMDS and MAX_PREALLOCATED_USER_PMDS are defined as
zero:

204 #else  /* !CONFIG_X86_PAE */
205 
206 /* No need to prepopulate any pagetable entries in non-PAE modes. */
207 #define PREALLOCATED_PMDS       0
208 #define MAX_PREALLOCATED_PMDS   0
209 #define PREALLOCATED_USER_PMDS   0
210 #define MAX_PREALLOCATED_USER_PMDS 0
211 #endif  /* CONFIG_X86_PAE */

It seems that GCC is legitimately complaining about the fact that, under
certain circumstances, u_pmds and pmds are declared as zero-length arrays
in the stack and, of course, they are not flexible arrays.

424 pgd_t *pgd_alloc(struct mm_struct *mm)
425 {
426         pgd_t *pgd;
427         pmd_t *u_pmds[MAX_PREALLOCATED_USER_PMDS];
428         pmd_t *pmds[MAX_PREALLOCATED_PMDS];
429

Notice that "Accessing elements of zero-length arrays declared in such
contexts is undefined and may be diagnosed."[1]

We can fix this by checking that MAX_PREALLOCATED_PMDS and MAX_PREALLOCATED_USER_PMDS
are different than zero, prior to passing u_pmds amd pmds as arguments to any
function, in this case to functions preallocate_pmds(), pgd_prepopulate_pmd()
and free_pmds().

This helps with the ongoing efforts to globally enable
-Wstringop-overflow.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

Link: https://github.com/KSPP/linux/issues/181
Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
---
Changes in v2:
 - Check MAX_PREALLOCATED_PMDS and MAX_PREALLOCATED_USER_PMDS
   instead of using pointer notation.
   Link: https://lore.kernel.org/linux-hardening/20220401005834.GA182932@embeddedor/
 - Update changelog text.

 arch/x86/mm/pgtable.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index f16059e9a85e..96c3f402a1da 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -434,14 +434,18 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 
 	mm->pgd = pgd;
 
-	if (preallocate_pmds(mm, pmds, PREALLOCATED_PMDS) != 0)
-		goto out_free_pgd;
+	if (MAX_PREALLOCATED_PMDS != 0 && MAX_PREALLOCATED_USER_PMDS != 0) {
+		if (preallocate_pmds(mm, pmds, PREALLOCATED_PMDS) != 0)
+			goto out_free_pgd;
 
-	if (preallocate_pmds(mm, u_pmds, PREALLOCATED_USER_PMDS) != 0)
-		goto out_free_pmds;
+		if (preallocate_pmds(mm, u_pmds, PREALLOCATED_USER_PMDS) != 0)
+			goto out_free_pmds;
 
-	if (paravirt_pgd_alloc(mm) != 0)
-		goto out_free_user_pmds;
+		if (paravirt_pgd_alloc(mm) != 0)
+			goto out_free_user_pmds;
+	} else {
+		goto out_free_pgd;
+	}
 
 	/*
 	 * Make sure that pre-populating the pmds is atomic with
-- 
2.27.0