All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning
@ 2021-09-28  9:34 Arnd Bergmann
  2021-09-28 10:17 ` Chris Down
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-09-28  9:34 UTC (permalink / raw)
  To: Petr Mladek, Sergey Senozhatsky
  Cc: Arnd Bergmann, Steven Rostedt, Chris Down, John Ogness,
	Nathan Chancellor, Nick Desaulniers, YueHaibing, Jessica Yu,
	Andy Shevchenko, linux-kernel, llvm

From: Arnd Bergmann <arnd@arndb.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")
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning
  2021-09-28  9:34 [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning Arnd Bergmann
@ 2021-09-28 10:17 ` Chris Down
  2021-10-04  9:22   ` Petr Mladek
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Down @ 2021-09-28 10:17 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Petr Mladek, Sergey Senozhatsky, Arnd Bergmann, Steven Rostedt,
	John Ogness, Nathan Chancellor, Nick Desaulniers, YueHaibing,
	Jessica Yu, Andy Shevchenko, linux-kernel, llvm

Arnd Bergmann writes:
>From: Arnd Bergmann <arnd@arndb.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@goodmis.org>
>Signed-off-by: Arnd Bergmann <arnd@arndb.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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning
  2021-09-28 10:17 ` Chris Down
@ 2021-10-04  9:22   ` Petr Mladek
  2021-10-04 12:31     ` Chris Down
  0 siblings, 1 reply; 4+ messages in thread
From: Petr Mladek @ 2021-10-04  9:22 UTC (permalink / raw)
  To: Chris Down
  Cc: Arnd Bergmann, Sergey Senozhatsky, Arnd Bergmann, Steven Rostedt,
	John Ogness, Nathan Chancellor, Nick Desaulniers, YueHaibing,
	Jessica Yu, Andy Shevchenko, linux-kernel, llvm

On Tue 2021-09-28 11:17:26, Chris Down wrote:
> Arnd Bergmann writes:
> > From: Arnd Bergmann <arnd@arndb.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.

IMHO, the sentence "Rework the condition to make it clear that this
is theoretical and will never actually happen" is rather clear.

Well, I am not a native speaker.

Anyway, I have pushed the patch into printk/linux.git, branch
for-5.16.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning
  2021-10-04  9:22   ` Petr Mladek
@ 2021-10-04 12:31     ` Chris Down
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Down @ 2021-10-04 12:31 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Arnd Bergmann, Sergey Senozhatsky, Arnd Bergmann, Steven Rostedt,
	John Ogness, Nathan Chancellor, Nick Desaulniers, YueHaibing,
	Jessica Yu, Andy Shevchenko, linux-kernel, llvm

Petr Mladek writes:
>On Tue 2021-09-28 11:17:26, Chris Down wrote:
>> Arnd Bergmann writes:
>> > From: Arnd Bergmann <arnd@arndb.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.
>
>IMHO, the sentence "Rework the condition to make it clear that this
>is theoretical and will never actually happen" is rather clear.

Sounds good to me, thanks!

I guess it's unneeded at this point, but feel free to add:

Acked-by: Chris Down <chris@chrisdown.name>

>Well, I am not a native speaker.
>
>Anyway, I have pushed the patch into printk/linux.git, branch
>for-5.16.
>
>Best Regards,
>Petr

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-04 12:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28  9:34 [PATCH] [v2] printk: avoid -Wsometimes-uninitialized warning Arnd Bergmann
2021-09-28 10:17 ` Chris Down
2021-10-04  9:22   ` Petr Mladek
2021-10-04 12:31     ` Chris Down

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.