linux-kernel - Re: [PATCH v2 0/5] timer_migration: Fix two possible races and an improvement

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] [day] [month] [year] [list]

Message-ID: <ZnoIlO22habOyQRe@lothringen>
Date: Tue, 25 Jun 2024 02:00:20 +0200
From: Frederic Weisbecker <frederic@...nel.org>
To: Anna-Maria Behnsen <anna-maria@...utronix.de>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2 0/5] timer_migration: Fix two possible races and an
 improvement

On Mon, Jun 24, 2024 at 04:53:52PM +0200, Anna-Maria Behnsen wrote:
> Borislav reported a warning in timer migration deactive path
> 
>   https://lore.kernel.org/r/20240612090347.GBZmlkc5PwlVpOG6vT@fat_crate.local
> 
> Sadly it doesn't reproduce directly. But with the change of timing (by
> adding a trace prinkt before the warning), it is possible to trigger the
> warning reliable at least in my test setup. The problem here is a racy
> check agains group->parent pointer. This is also used in other places in
> the code and fixing this racy usage is adressed by the first patch.
> 
> There was another race reported by Frederic in setup path:
> 
>   https://lore.kernel.org/r/ZnWOswTMML6ShzYO@localhost.localdomain
> 
> It is addressed patch 2-4. Patch 2 is an already existing patch of v1
> (improve tracing) and makes the fix easier. Patch 3 is also a preparation
> patch for the final fix and Patch 4 is then the real fix. (I labelled all
> those three patches with Fixes tag to be easier selectable.)
> 
> While working with the code, I saw that the update of per cpu group wakeup
> value could be improved. This improvement is adressed by the last patch.

Another possible issue with the CPU up code is the fact that child->parent and
child->childmask updates and reads are unordered.

So it's possible that a CPU going (in-)active sees the new parent but doesn't
observe yet the update to child->childmask. This may result to calling
tmigr_(in-)active_up() with a zero data->childmask with all sorts of
consequences.

Or am I missing something that prevents from that?

Thanks.