linux-kernel - Re: [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list

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: <20181015224614.GB30658@n2100.armlinux.org.uk>
Date:   Mon, 15 Oct 2018 23:46:15 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Stefan Agner <stefan@...er.ch>
Cc:     raj.khem@...il.com, ulli.kroll@...glemail.com, joel@....id.au,
        nico@...aro.org, arnd@...db.de, linus.walleij@...aro.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input
 operands list

On Tue, Oct 16, 2018 at 12:39:54AM +0200, Stefan Agner wrote:
> On 16.10.2018 00:23, Russell King - ARM Linux wrote:
> > On Tue, Oct 16, 2018 at 12:16:29AM +0200, Stefan Agner wrote:
> >> When functions incoming parameters are not in input operands list gcc
> >> 4.5 does not load the parameters into registers before calling this
> >> function but the inline assembly assumes valid addresses inside this
> >> function. This breaks the code because r0 and r1 are invalid when
> >> execution enters v4wb_copy_user_page ()
> > 
> > NAK.  Naked functions must never be inlined.  Please add a "noinline"
> > attribute to the function rather than making things more complex.
> > 
> 
> To be honest, I did not put much thought into this commit since it is
> just doing to copypage-fa.c what 9a40ac86152c ("ARM: 6164/1: Add kto and
> kfrom to input operands list.") has been done to the other copypage
> implementations...
> 
> [adding Khem]
> 
> > The GCC manual states:
> > 
> > `naked'
> >      Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX
> >      and SPU ports to indicate that the specified function does not
> >      need prologue/epilogue sequences generated by the compiler.  It is
> >      up to the programmer to provide these sequences. The only
> >                                                       ^^^^^^^^
> >      statements that can be safely included in naked functions are
> >      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >      `asm' statements that do not have operands.  All other statements,
> >      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >      including declarations of local variables, `if' statements, and so
> >      forth, should be avoided.  Naked functions should be used to
> >      implement the body of an assembly function, while allowing the
> >      compiler to construct the requisite function declaration for the
> >      assembler.
> > 
> > The 'I' attribute is fine here because it is a constant that is not
> > allowed to be in a register (and hence has no code generation side
> > effects.)
> > 
> > Adding operands for the input parameters, however, isn't going to
> > work around the fact that _this_ assembly is written to be out of
> > line and so it must never be inlined by the compiler.
> 
> I briefly looked at a disassembled version after applying both patches,
> it indeed leads to inlining. However, the code seems to be working
> (thanks to asm volatile?)...

Apart from v4wb_copy_user_page() and mc_copy_user_page(), how is
Clang inlining these static functions that are only used through
function pointers?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up