linux-kernel - Re: [PATCH] x86/irqflags: mark native_restore

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:   Tue, 28 Aug 2018 15:41:55 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     jgross@...e.com, Thomas Gleixner <tglx@...utronix.de>,
        mingo@...hat.com, stable@...r.kernel.org, hpa@...or.com,
        x86@...nel.org, boris.ostrovsky@...cle.com,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/irqflags: mark native_restore_fl extern inline

On Tue, Aug 28, 2018 at 5:43 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Tue, Aug 28, 2018 at 08:02:37AM +0200, Juergen Gross wrote:
> > On 28/08/18 07:13, Greg Kroah-Hartman wrote:
> > > On Mon, Aug 27, 2018 at 02:40:09PM -0700, Nick Desaulniers wrote:
> > >> Fixes commit 208cbb325589 ("x86/irqflags: Provide a declaration for
> > >> native_save_fl")
> > >>
> > >> This should have been marked extern inline in order to pick up the out
> > >> of line definition in arch/x86/kernel/irqflags.S.
> > >>
> > >> Cc: stable@...r.kernel.org # 4.18, 4.14, 4.9, 4.4
> > >> Reported-by: Ben Hutchings <ben.hutchings@...ethink.co.uk>
> > >> Signed-off-by: Nick Desaulniers <ndesaulniers@...gle.com>
> > >> ---
> > >>  arch/x86/include/asm/irqflags.h | 3 ++-
> > >>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
> > >> index c14f2a74b2be..15450a675031 100644
> > >> --- a/arch/x86/include/asm/irqflags.h
> > >> +++ b/arch/x86/include/asm/irqflags.h
> > >> @@ -33,7 +33,8 @@ extern inline unsigned long native_save_fl(void)
> > >>    return flags;
> > >>  }
> > >>
> > >> -static inline void native_restore_fl(unsigned long flags)
> > >> +extern inline void native_restore_fl(unsigned long flags);
> > >> +extern inline void native_restore_fl(unsigned long flags)
> > >
> > > This looks odd to me, but my coffee hasn't kicked in yet this morning.
> > > Why do you need both lines here?  Shouldn't the actual function be
> > > sufficient?  If not, a comment explaining this would be nice.

That's a valid concern.  I did hesitate while writing the commit
message whether to describe that part or not.  The reason to include
the seemingly-additional-declaration was to prevent another case of
commit 208cbb325589 ("x86/irqflags: Provide a declaration for
native_save_fl").  It doesn't hurt to provide more info in the commit
message, so next time I'll prefer to explain more in my commit
messages than less.

Thanks Juergen and Greg for reviewing the patch.

-- 
Thanks,
~Nick Desaulniers