linux-kernel - Re: native_save_fl() causes a 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]

Date:   Fri, 3 Aug 2018 09:38:03 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     jdelvare@...e.de
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Alistair Strachan <astrachan@...gle.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Arnd Bergmann <arnd@...db.de>, hpa@...or.com,
        tstellar@...hat.com, sedat.dilek@...il.com, jgross@...e.com,
        Ingo Molnar <mingo@...nel.org>, David.Laight@...lab.com
Subject: Re: native_save_fl() causes a warning

On Fri, Aug 3, 2018 at 6:10 AM Jean Delvare <jdelvare@...e.de> wrote:
>
> Hi Nick,
>
> It seems that this linux kernel commit of yours:
>
> commit d0a8d9378d16eb3c69bd8e6d23779fbdbee3a8c7
> Author: Nick Desaulniers
> Date:   Thu Jun 21 09:23:24 2018 -0700
>
>     x86/paravirt: Make native_save_fl() extern inline
>
> introduced a new warning (with W=1):
>
> ./arch/x86/include/asm/irqflags.h:16:29: warning: no previous prototype for ‘native_save_fl’ [-Wmissing-prototypes]
>  extern inline unsigned long native_save_fl(void)
>                              ^
>
> Please fix it.

Hi Jean, thanks for the report.  David Laight also reported this
warning; he tested a patch I sent him overnight.

Let me guess, you're using a version of GCC < 4.9?  It seems that GCC
< 4.9 will produce that warning for extern inline functions without
previous declarations.

I'll add your Reported-By tag to the patch that I will send out in a
few minutes.

> Secondly, I am quite curious why you changed only native_save_fl() from
> static inline to extern inline, when native_restore_fl(),
> native_irq_disable() and native_irq_enable() are equally referenced by
> address in arch/x86/kernel/paravirt.c and thus should suffer from the
> same problem. Can you explain?

This is a good point.  With native_save_fl, we were not able to boot
the kernel at all.  Maybe this was called from the boot sequence
(maybe Juergen knows more)?  It seems that the other functions aren't
preventing us from booting, but maybe exercising other paths in
paravirt would expose such an issue?  Or maybe paravirt doesn't have
the same calling convention requirements for those functions?

Is there a standard testing procedure for paravirt? I'd be happy to
try it to see if we can expose more things that should have the same
cleanup.
-- 
Thanks,
~Nick Desaulniers