linux-kernel - Re: [GIT pull] x86/urgent for v5.8-rc4

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

Message-Id: <B8CAA214-A115-4852-9AAC-0A68CD0FA76E@amacapital.net>
Date:   Sun, 5 Jul 2020 13:23:17 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: Re: [GIT pull] x86/urgent for v5.8-rc4






--Andy
> On Jul 5, 2020, at 12:46 PM, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> On Sun, Jul 5, 2020 at 8:47 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>> 
>> - Disable 16 bit segments on XEN PV. It's not supported because XEN PV
>>   does not implement ESPFIX64
> 
> I don't disagree with this conceptually, and I've pulled it, but
> christ, that warning is over-engineered.
> 
> Seriously, it uses a mutex to protect a "set once" variable. That's just crazy.
> 
> We have "pr_info_once()", which does all of this for you. And no, it's
> not thread-safe, becasue ABSOLUTELY NOBODY CARES.
> 
> If you happen to get two or more warnings because they happen on
> separate CPU's at exactly the same time, nobody possibly cares.
> 
> And if you really are so anal that you care about that case, using a
> mutex is still the wrong thing to do for something as simple as this.
> 
> You could literally have made it just a single atomic variable, and
> that would have been simpler, faster, and be context-safe at the same
> time.
> 
> So using a mutex is not only overkill, it is literally technically
> _inferior_ to just about all the possible ways you can do this.
> 
> I've pulled this, but I found that code so bad as to be actually
> offensive, and added a commit to remove the garbage and just use
> "pr_info_once()".
> 
> And if somebody wants to guarantee the "it really can only happen once
> even in theory", that person can add the code to "pr_info_once()" to
> improve it to first optimistically load the value, and then use a
> "cmpxchg" or whatever.
> 
> But that sounds pointless, and I'm not going to waste my time on it.
> 
> But I _did_ waste my time on removing this horrendous case of
> re-implementing "pr_info_once()" horribly badly.
> 
> Because it physically hurt my eyes to look at that code.


I’m not sure whether I should apologize or feel proud of my ability to injure your eyeballs at a distance.  In my defense, I looked for helpers like this by grepping for _ONCE and didn’t find _once.  I suppose I should have remembered that this helper existed.

Thanks for the fixup!