linux-kernel - Re: linux-next: build warnings after merge of the 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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Mar 2015 09:21:50 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Will Deacon <will.deacon@....com>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	"linux-next@...r.kernel.org" <linux-next@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>
Subject: Re: linux-next: build warnings after merge of the access_once tree

On Thu, Mar 26, 2015 at 9:15 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> Really. Just get rid of the checks - they were wrong. They were
> clearly very close to *introducing* a bug, rather than fixing anything
> at all.

Side note: we will continue to expect the compiler to do single-word
accesses as a single acccess, rather than splitting things up. And
that's fine. READ_ONCE() uses "volatile", and it means on a language
level that the actual access is "visible", so it's a reasonable
expectation to have.

So the proper patch looks something like this:

    diff --git a/include/linux/compiler.h b/include/linux/compiler.h
    index 1b45e4a0519b..f36e1abf56ea 100644
    --- a/include/linux/compiler.h
    +++ b/include/linux/compiler.h
    @@ -198,10 +198,6 @@ __compiletime_warning("data access exceeds
word size and won't be atomic")
     #endif
     ;

    -static __always_inline void data_access_exceeds_word_size(void)
    -{
    -}
    -
     static __always_inline void __read_once_size(const volatile void
*p, void *res, int size)
     {
             switch (size) {
    @@ -214,7 +210,6 @@ static __always_inline void
__read_once_size(const volatile void *p, void *res,
             default:
                     barrier();
                     __builtin_memcpy((void *)res, (const void *)p, size);
    -                data_access_exceeds_word_size();
                     barrier();
             }
     }
    @@ -231,7 +226,6 @@ static __always_inline void
__write_once_size(volatile void *p, void *res, int s
             default:
                     barrier();
                     __builtin_memcpy((void *)p, (const void *)res, size);
    -                data_access_exceeds_word_size();
                     barrier();
             }
     }

and let's just leave it at that.

Note again how the default case just guarantees that it is never
reloaded by the compiler. That's the primary issue this is all about.
The whole "we expect the compiler to not be shit" is secondary and
_may_ be an issue in some places, but is not what the main goal is.

                             Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/