linux-kernel - Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20)

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]

Date:	Fri, 15 Jul 2016 14:29:31 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:	linaro-kernel@...ts.linaro.org,
	Build bot for Mark Brown <broonie@...nel.org>,
	kernel-build-reports@...ts.linaro.org,
	linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20)

On Friday, July 15, 2016 2:05:27 PM CEST Sebastian Andrzej Siewior wrote:
> * Arnd Bergmann | 2016-07-15 09:07:20 [+0200]:
> 
> >Cc linux-rt-users@...r.kernel.org, let's have a look at the individual warnings:
> thanks.
> 
> >
> >> Warnings Summary: 5
> >> 	  9 ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
> >
> >This was introduced by the rt patchset, I'd suggest adding a fix to the next
> >v4.4-rt release. This is almost certainly harmless.
> 
> has been adressed in 4.6.4-rt8 and is harmless.

Ok.

> |drivers/tty/serial/amba-pl011.c: In function ‘pl011_console_write’:
> |include/linux/spinlock.h:246:3: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> |   _raw_spin_unlock_irqrestore(lock, flags); \
> |   ^
> |drivers/tty/serial/amba-pl011.c:2065:16: note: ‘flags’ was declared here
> |  unsigned long flags;
> |                ^
> 
> and relevant part of the source:
> |pl011_console_write(struct console *co, const char *s, unsigned int count)
> |{
> …
> |        unsigned long flags;
> |        int locked = 1;
> |
> …
> |        if (uap->port.sysrq)
> |                locked = 0;
> |        else if (oops_in_progress)
> |                locked = spin_trylock_irqsave(&uap->port.lock, flags);
> |        else
> |                spin_lock_irqsave(&uap->port.lock, flags);
> …
> |        if (locked)
> |                spin_unlock_irqrestore(&uap->port.lock, flags);
> …
> |}
> 
> looks like a false positive to me.

Agreed, but it's hard for the compiler to figure that out. On mainline,
we have this instead:

       local_irq_save(flags);
       if (uap->port.sysrq)
               locked = 0;
       else if (oops_in_progress)
               locked = spin_trylock(&uap->port.lock);
       else
               spin_lock(&uap->port.lock);
…
       if (locked)
               spin_unlock(&uap->port.lock);
       local_irq_restore(flags);

which looks like it's intentionally written to avoid the warning
and was changed by Thomas in "tty/serial/pl011: Make the locking work on RT":

http://git.kernel.org/cgit/linux/kernel/git/rt/linux-stable-rt.git/commit/drivers/tty/serial/amba-pl011.c?h=v4.4-rt-rebase&id=7b537b66fcb12c40eb1b10eb352d570a9d34a657

Maybe there is another way to write this upstream that avoids the
warning.

	Arnd