linux-kernel - Re: [PATCH] arch/openrisc: Fix issues with access

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] [day] [month] [year] [list]

Date:   Wed, 9 Jan 2019 07:37:48 +0900
From:   Stafford Horne <shorne@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Jonas Bonn <jonas@...thpole.se>,
        Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
        openrisc@...ts.librecores.org
Subject: Re: [PATCH] arch/openrisc: Fix issues with access_ok()

On Tue, Jan 08, 2019 at 10:16:39AM -0800, Linus Torvalds wrote:
> On Tue, Jan 8, 2019 at 10:10 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > On Tue, Jan 8, 2019 at 5:15 AM Stafford Horne <shorne@...il.com> wrote:
> > >
> > > The commit 594cc251fdd0 ("make 'user_access_begin()' do 'access_ok()'")
> > > exposed incorrect implementations of access_ok() macro in several
> > > architectures.  This change fixes 2 issues found in OpenRISC.
> >
> > Looks good to me. Should I apply this directly, or expect a pull
> > request with it later?
> 
> Oh, and replying to myself with a quick note: it might also be a good
> idea to just make it an inline function.
> 
> The only reason I did alpha and SH as those macros with a statement
> expression was that because I don't have a cross-build environment,
> continuing to do it as a macro was the safest thing from a build
> standpoint.
> 
> One big difference between a macro and an inline function is that the
> inline function requires everything to be declared at the point of the
> function definition, while the macro can use things that get declared
> only later (like "get_fs()"). So a macro can use functions and other
> macros that aren't declared yet, but will be declared by the time the
> macro is actually _used_.
> 
> So when changing the macro "blind", it was simply safer to just keep
> it a macro and just make it a bit more complex. But since you can
> build-test your changes, making (for example) __range_ok() be an
> inline function might have been the cleaner solution to the "use
> twice" issue.
> 
> But your existing patch looks fine to me too, so don't worry too much
> about it. I just wanted to point out that the reason I did alpha and
> SH the way I did was not some "macros are better", but rather "Linus
> is weak and lazy".

Hi Linus,

Please take this patch directly.

The inline's are a good point.  I will take some time to address this and some
other cleanups.

Note (for others) I had to apply patches from Masahiro Yamada to test this as
v5.0-rc1 build was broken for OpenRISC.  Those patches are already on Linus'
master.

-Stafford