linux-kernel - Re: [GIT PULL] locking fixes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 Feb 2015 17:51:56 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ingo Molnar <mingo@...nel.org>,
	Christian Borntraeger <borntraeger@...ibm.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] locking fixes

So here's my try at fixing READ_ONCE() so that it is happy with 'const' sources.

It is entirely untested. Comments/testing?

Christian, I guess I could just have forced a cast instead of the
union. I'd like you to take a look at this, because right now it's
holding up me pulling from Ingo.

And Ingo, I think you need to add some kind of test for "horrible new
warnings". I think your pull request *worked*, but the tens of lines
of new warnings it generates is unacceptable, and will just cause me
to undo the pull if I notice in time (like I did this time).

                            Linus

On Fri, Feb 20, 2015 at 4:03 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> How does this work for you at all?
>
> On Fri, Feb 20, 2015 at 5:37 AM, Ingo Molnar <mingo@...nel.org> wrote:
>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>> index 94f643484300..e354cc6446ab 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -803,8 +808,8 @@ static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket)
>>         add_stats(RELEASED_SLOW, 1);
>>         for_each_cpu(cpu, &waiting_cpus) {
>>                 const struct kvm_lock_waiting *w = &per_cpu(klock_waiting, cpu);
>> -               if (ACCESS_ONCE(w->lock) == lock &&
>> -                   ACCESS_ONCE(w->want) == ticket) {
>> +               if (READ_ONCE(w->lock) == lock &&
>> +                   READ_ONCE(w->want) == ticket) {
>>                         add_stats(RELEASED_SLOW_KICKED, 1);
>>                         kvm_kick_cpu(cpu);
>>                         break;
>
> I get horrible compile warnings from this, because of how 'w' is a
> pointer to a 'const' structure, which then causes things like
>
>     include/linux/compiler.h:262:39: warning: passing argument 1 of
> ‘__read_once_size’ discards ‘const’ qualifier from pointer target type
>       ({ typeof(x) __val; __read_once_size(&x, &__val, sizeof(__val)); __val; })
>
> which is fairly hard to avoid (looks like it might need a union)
>
>                        Linus

View attachment "patch.diff" of type "text/plain" (1509 bytes)