linux-kernel - Re: BUG_ON() in workingset_node_shadows

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:   Wed, 5 Oct 2016 09:10:56 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Antonio SJ Musumeci <trapexit@...wn.link>,
        Miklos Szeredi <miklos@...redi.hu>,
        Dave Jones <davej@...emonkey.org.uk>,
        Oleg Nesterov <oleg@...hat.com>,
        Dave Chinner <david@...morbit.com>,
        Michal Hocko <mhocko@...nel.org>, Jan Kara <jack@...e.cz>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: BUG_ON() in workingset_node_shadows_dec() triggers

On Wed, Oct 5, 2016 at 2:25 AM, Johannes Weiner <hannes@...xchg.org> wrote:
>
> Here is a reproducer that triggers the warning instantly for me:

Yup, confirmed.With the VM_WARN_ON_ONCE() it just gets a big nice
splat and the machine happily stays up.

> That radix tree node management needs some cleaning up. It probably
> makes sense to split node->count into actually separate members for
> clarity, and then add a root tag to distinguish shadows from regular
> entries in root->rnode. I have to think about this more, the current
> situation is too fragile and ugly.

Ugh. I even looked at the "node->count = 1" initialization in
radix_tree_extend(), and didn't react to it at all, it looked
obviously correct.

This code is too subtle.

> But in the meantime, there is an obvious fix: don't ever store shadow
> entries in root->rnode, seeing as we need nodes for proper accounting.
>
> It means we temporarily lose the ability to detect refaults from
> single-page files, but it's probably better to keep the stable fix
> small and restore that functionality in a new release.
>
> Patch below. NOTE: I'm traveling without access to my test rig right
> now and so I have only lightly tested this on my laptop. I'm also
> jetlagged like crazy, so please triple check my thinking. The patch
> does fix the reproducer case and has otherwise been stable here.

Hmm. I'm inclined to just apply it and mark it for stable, along with
your other patch. But yes, this needs more thinking about (and
obviously testing). The interactions with the radix tree are too
subtle.

                               Linus