linux-kernel - Re: [PATCH 2/2] random: spread out jitter callback to different CPUs

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: <Yz7M5zJmzKSk/LYH@linutronix.de>
Date:   Thu, 6 Oct 2022 14:41:11 +0200
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Sultan Alsawaf <sultan@...neltoast.com>
Subject: Re: [PATCH 2/2] random: spread out jitter callback to different CPUs

On 2022-10-06 06:26:04 [-0600], Jason A. Donenfeld wrote:
> e) del_timer_sync() on line 5 is called, and its `base->running_timer !=
>    timer` check is false, because of step (c).

If `base->running_timer != timer` then the timer ('s callback) is not
currently active/ running. Therefore it can be removed from the timer
bucket (in case it is pending in the future).
If `base->running_timer == timer` then the timer ('s callback) is
currently active. del_timer_sync() will loop in cpu_relax() until the
callback finished. And then try again.

> f) `stack.timer` gets freed / goes out of scope.
> 
> g) The callback scheduled from step (b) runs, and we have a UaF.
> 
> That's, anyway, what I understand Sultan to have pointed out to me. In
> looking at this closely, though, to write this email, I noticed that
> add_timer_on() does set TIMER_MIGRATING, which lock_timer_base() spins
> on. So actually, maybe this scenario should be accounted for? Sultan, do
> you care to comment here?

During TIMER_MIGRATING the del_timer_sync() caller will spin until the
condition is over. So it can remove the timer from the right bucket and
check if it is active vs the right bucket.

> Jason

Sebastian