linux-kernel - Re: [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning

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: <YVLrttKajDU+1ZvX@chrisdown.name>
Date:   Tue, 28 Sep 2021 11:17:26 +0100
From:   Chris Down <chris@...isdown.name>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        Steven Rostedt <rostedt@...dmis.org>,
        John Ogness <john.ogness@...utronix.de>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        YueHaibing <yuehaibing@...wei.com>, Jessica Yu <jeyu@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning

Arnd Bergmann writes:
>From: Arnd Bergmann <arnd@...db.de>
>
>clang notices that the pi_get_entry() function would use
>uninitialized data if it was called with a non-NULL module
>pointer on a kernel that does not support modules:
>
>kernel/printk/index.c:32:6: error: variable 'nr_entries' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>        if (!mod) {
>            ^~~~
>kernel/printk/index.c:38:13: note: uninitialized use occurs here
>        if (pos >= nr_entries)
>                   ^~~~~~~~~~
>kernel/printk/index.c:32:2: note: remove the 'if' if its condition is always true
>        if (!mod) {
>
>Rework the condition to make it clear to the compiler that we are always
>in the second case. Unfortunately the #ifdef is still required as the
>definition of 'struct module' is hidden when modules are disabled.
>
>Fixes: 337015573718 ("printk: Userspace format indexing support")

This changelog should make it clear that this is theoretical and will never 
actually happen, which is salient information for people who are considering 
whether it should go in stable or similar.

>Suggested-by: Steven Rostedt <rostedt@...dmis.org>
>Signed-off-by: Arnd Bergmann <arnd@...db.de>
>--
>v2: use a simpler trick of having an 'else' in the #ifdef
>    block, as Steven suggested.
>---
> kernel/printk/index.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/kernel/printk/index.c b/kernel/printk/index.c
>index d3709408debe..43b45a916ff6 100644
>--- a/kernel/printk/index.c
>+++ b/kernel/printk/index.c
>@@ -26,10 +26,9 @@ static struct pi_entry *pi_get_entry(const struct module *mod, loff_t pos)
> 	if (mod) {
> 		entries = mod->printk_index_start;
> 		nr_entries = mod->printk_index_size;
>-	}
>+	} else
> #endif
>-
>-	if (!mod) {
>+	{
> 		/* vmlinux, comes from linker symbols */
> 		entries = __start_printk_index;
> 		nr_entries = __stop_printk_index - __start_printk_index;
>-- 
>2.29.2
>