linux-kernel - Re: [Resend PATCH v6] psi: fix race between psi_trigger

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]

Date:   Fri, 11 Jun 2021 08:58:31 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Zhaoyang Huang <huangzhaoyang@...il.com>
Cc:     Johannes Weiner <hannes@...xchg.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Zhaoyang Huang <zhaoyang.huang@...soc.com>,
        Ziwei Dai <ziwei.dai@...soc.com>, Ke Wang <ke.wang@...soc.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>
Subject: Re: [Resend PATCH v6] psi: fix race between
 psi_trigger_create/destroy

On Fri, Jun 11, 2021 at 08:37:05AM +0800, Zhaoyang Huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> 
> Race detected between psi_trigger_destroy/create as shown below, which
> cause panic by accessing invalid psi_system->poll_wait->wait_queue_entry
> and psi_system->poll_timer->entry->next. Under this modification, the
> race window is removed by initialising poll_wait and poll_timer in
> group_init which are executed only once at beginning.
> 
> psi_trigger_destroy                      psi_trigger_create
> mutex_lock(trigger_lock);
> rcu_assign_pointer(poll_task, NULL);
> mutex_unlock(trigger_lock);
>                                         mutex_lock(trigger_lock);
>                                         if
> (!rcu_access_pointer(group->poll_task)) {
> 
> 
> timer_setup(poll_timer, poll_timer_fn, 0);
> 
> 
> rcu_assign_pointer(poll_task, task);
>                                         }
>                                         mutex_unlock(trigger_lock);
> 
> synchronize_rcu();
> del_timer_sync(poll_timer); <-- poll_timer has been reinitialized by
> psi_trigger_create
> 
> So, trigger_lock/RCU correctly protects destruction of group->poll_task but
> misses this race affecting poll_timer and poll_wait.
> 
> Fixes: 461daba06bdc ("psi: eliminate kthread_worker from psi trigger
> scheduling mechanism")
> 
> Co-developed-by: ziwei.dai <ziwei.dai@...soc.com>
> Signed-off-by: ziwei.dai <ziwei.dai@...soc.com>
> Co-developed-by: ke.wang <ke.wang@...soc.com>
> Signed-off-by: ke.wang <ke.wang@...soc.com>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> ---

You really should've preserved the tags from Suren and Johannes, I've
added them. Also the Fixes: line shouldn't wrap and should be attached
to the other tags (no whitespace between), also fixed that. And I've
also made another few small edits.

Please pay attention to these things for next time.

Patch can be found in my queue and should show in tip/sched/core
somewhere on Monday if the robots don't hate on it.