linux-kernel - Re: [PATCH-tip v6 01/20] locking/rwsem: Prevent decrement of reader count before increment

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: <CAHk-=wgGN9KKCW8dii=nXwKQdu2N5AOMrVdS5qJvZAPOTvwHjA@mail.gmail.com>
Date:   Sun, 28 Apr 2019 10:59:06 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Waiman Long <longman@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Will Deacon <will.deacon@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        huang ying <huang.ying.caritas@...il.com>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH-tip v6 01/20] locking/rwsem: Prevent decrement of reader
 count before increment

On Sun, Apr 28, 2019 at 10:41 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> It's the *first* loop that you could play games with, because you hold
> the lock, and the list is stable during that loop. So the *first* loop
> could just walk the list, and then do one list splitting operation
> instead of doing that "list_move_tail()" thing for each entry.

.. having looked at that, I would suggest against it.

I _think_ this short and sweet code snippet might just work fine for
the first loop:

        list_for_each_entry(waiter, &sem->wait_list, list) {
                if (waiter->type == RWSEM_WAITING_FOR_WRITE)
                        break;
                woken++;
        }
        list_cut_before(&wlist, &sem->wait_list, waiter);

and if it *does* work it would be both smaller and more efficient. But
it looks a bit too subtle to my taste. Somebody would need to go
through that with a fine comb, and double-check that it gets the
"whole list" case right, for example.

So the "phase 1" loop could be perhaps simplified to the above cute things.

But the "phase 2" loop absolutely has to be changed to use
list_for_each_entry_safe().

               Linus