linux-kernel - Re: [mm/gup] 9857a17f20: kernel_BUG_at

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:   Tue, 7 Sep 2021 11:14:56 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     kernel test robot <oliver.sang@...el.com>
Cc:     John Hubbard <jhubbard@...dia.com>, Christoph Hellwig <hch@....de>,
        Matthew Wilcox <willy@...radead.org>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>, lkp@...ts.01.org,
        kernel test robot <lkp@...el.com>
Subject: Re: [mm/gup] 9857a17f20: kernel_BUG_at_include/linux/pagemap.h

On Tue, Sep 7, 2021 at 8:20 AM kernel test robot <oliver.sang@...el.com> wrote:
>
> FYI, we noticed the following commit (built with clang-14):
>
> commit: 9857a17f206f ("mm/gup: remove try_get_page(), call try_get_compound_head() directly")
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
> [  143.908513][ T3260] kernel BUG at include/linux/pagemap.h:223!

Ahh, well, yes.

That commit is clearly buggy, in that the try_get_compound_head() code
really doesn't work at all for us.

__page_cache_add_speculative() is not at all the same as
try_get_page(), and I should have caught on to this as I applied it. I
just read the explanation, and it sounded believable, but it was
entirely wrong.

try_get_page() is literally about that "page ref overflow" case, but
try_get_compound_head() uses page_cache_add_speculative() which has
different logic and has those extra "this only works in RCU context"
logic.

So that commit was completely bogus, and the "lack of maintenance" was
not lack of maintenance at all, it was all about entirely different
semantics.

Reverted.

                 Linus