linux-kernel - Re: [PATCH v2 4/5] timer_migration: Fix possible race in tmigr_active

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: <ZnnyJPPTm8NHqPbf@pavilion.home>
Date: Tue, 25 Jun 2024 00:24:36 +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
Subject: Re: [PATCH v2 4/5] timer_migration: Fix possible race in
 tmigr_active_up() in setup path

Le Tue, Jun 25, 2024 at 12:01:27AM +0200, Frederic Weisbecker a écrit :
> > +static void tmigr_setup_active_up(struct tmigr_group *group, struct tmigr_group *child)
> > +{
> > +	union tmigr_state curstate, childstate;
> > +	bool walk_done;
> > +
> > +	/*
> > +	 * Memory barrier is paired with the cmpxchg in tmigr_inactive_up() to
> > +	 * make sure updates of child and group states are ordered. The ordering
> > +	 * is mandatory, as the update of the group state is only valid for when
> > +	 * child state is active.
> > +	 */
> > +	curstate.state = atomic_read_acquire(&group->migr_state);
> > +
> > +	for (;;) {
> 
> /*
>  * If the child hasn't yet propagated anything to the top level, the above
>  * acquire has no effect. However thanks to child locking (see comment in
>  * tmigr_connect_child_parent()), either the latest child->migr_state is
>  * observed or the remote CPU now observes the new parent and is about to
>  * propagate to the new parent. In the latter case it will either beat the
>  * current trial update or overwrite it.
>  */
> 
> And another comment later:
> 
> > +		childstate.state = atomic_read(&child->migr_state);
> > +		if (!childstate.active)
> > +			return;

Hmm, on the other hand the remote group doesn't use locking if some tmc activates
after that and so there is no guarantee it will see the new parent. Does it
matter?

Thanks.