* Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
gcc-11 warns about using string operations on pointers that are defined at compile time as offsets from a NULL pointer. Unfortunately that also happens on the result of fix_to_virt(), which is a compile-time constant for a constantn input:
arch/x86/kernel/tboot.c: In function 'tboot_probe': arch/x86/kernel/tboot.c:70:13: error: '__builtin_memcmp_eq' specified bound 16 exceeds source size 0 [-Werror=stringop-overread] 70 | if (memcmp(&tboot_uuid, &tboot->uuid, sizeof(tboot->uuid))) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I hope this can get addressed in gcc-11 before the release.
As a workaround, split up the tboot_probe() function in two halves to separate the pointer generation from the usage. This is a bit ugly, and hopefully gcc understands that the code is actually correct before it learns to peek into the noinline function.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/x86/kernel/tboot.c | 44 ++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 4c09ba110204..f9af561c3cd4 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -49,6 +49,30 @@ bool tboot_enabled(void) return tboot != NULL; }
+/* noinline to prevent gcc from warning about dereferencing constant fixaddr */ +static noinline __init bool check_tboot_version(void) +{
- if (memcmp(&tboot_uuid, &tboot->uuid, sizeof(tboot->uuid))) {
pr_warn("tboot at 0x%llx is invalid\n", boot_params.tboot_addr);
return false;
- }
- if (tboot->version < 5) {
pr_warn("tboot version is invalid: %u\n", tboot->version);
return false;
- }
- pr_info("found shared page at phys addr 0x%llx:\n",
boot_params.tboot_addr);
- pr_debug("version: %d\n", tboot->version);
- pr_debug("log_addr: 0x%08x\n", tboot->log_addr);
- pr_debug("shutdown_entry: 0x%x\n", tboot->shutdown_entry);
- pr_debug("tboot_base: 0x%08x\n", tboot->tboot_base);
- pr_debug("tboot_size: 0x%x\n", tboot->tboot_size);
- return true;
+}
void __init tboot_probe(void) { /* Look for valid page-aligned address for shared page. */ @@ -66,25 +90,9 @@ void __init tboot_probe(void)
/* Map and check for tboot UUID. */ set_fixmap(FIX_TBOOT_BASE, boot_params.tboot_addr);
- tboot = (struct tboot *)fix_to_virt(FIX_TBOOT_BASE);
- if (memcmp(&tboot_uuid, &tboot->uuid, sizeof(tboot->uuid))) {
pr_warn("tboot at 0x%llx is invalid\n", boot_params.tboot_addr);
- tboot = (void *)fix_to_virt(FIX_TBOOT_BASE);
- if (!check_tboot_version()) tboot = NULL;
return;
- }
- if (tboot->version < 5) {
pr_warn("tboot version is invalid: %u\n", tboot->version);
tboot = NULL;
return;
- }
- pr_info("found shared page at phys addr 0x%llx:\n",
boot_params.tboot_addr);
- pr_debug("version: %d\n", tboot->version);
- pr_debug("log_addr: 0x%08x\n", tboot->log_addr);
- pr_debug("shutdown_entry: 0x%x\n", tboot->shutdown_entry);
- pr_debug("tboot_base: 0x%08x\n", tboot->tboot_base);
- pr_debug("tboot_size: 0x%x\n", tboot->tboot_size);
This is indeed rather ugly - and the other patch that removes a debug check seems counterproductive as well.
Do we know how many genuine bugs -Wstringop-overread-warning has caught or is about to catch?
I.e. the real workaround might be to turn off the -Wstringop-overread-warning, until GCC-11 gets fixed?
Thanks,
Ingo