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: <YddnuSh15BAGdjAD@slm.duckdns.org>
Date:   Thu, 6 Jan 2022 12:05:45 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Eric Biggers <ebiggers@...nel.org>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Ingo Molnar <mingo@...hat.com>,
        Hillf Danton <hdanton@...a.com>,
        syzbot <syzbot+cdb5dd11c97cc532efad@...kaller.appspotmail.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: psi_trigger_poll() is completely broken

Happy new year,

On Wed, Jan 05, 2022 at 11:13:30AM -0800, Linus Torvalds wrote:
> On Wed, Jan 5, 2022 at 11:07 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > Whoever came up with that stupid "replace existing trigger with a
> > write()" model should feel bad. It's garbage, and it's actively buggy
> > in multiple ways.
> 
> What are the users? Can we make the rule for -EBUSY simply be that you
> can _install_ a trigger, but you can't replace an existing one (except
> with NULL, when you close it).

I don't have enough context here and Johannes seems offline today. Let's
wait for him to chime in.

> That would fix the poll() lifetime issue, and would make the
> psi_trigger_replace() races fairly easy to fix - just use
> 
>         if (cmpxchg(trigger_ptr, NULL, new) != NULL) {
>                 ... free 'new', return -EBUSY ..
> 
> to install the new one, instead of
> 
>         rcu_assign_pointer(*trigger_ptr, new);
> 
> or something like that. No locking necessary.
> 
> But I assume people actually end up re-writing triggers, because
> people are perverse and have taken advantage of this completely broken
> API.

IIRC, the rationale for the shared trigger at the time was around the
complexities of preventing it from devolving into O(N) trigger checks on
every pressure update. If the overriding behavior is something that can be
changed, I'd prefer going for per-opener triggers even if that involves
adding complexities (maybe a rbtree w/ prev/next links for faster sweeps?).

Thanks.

-- 
tejun

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ