linux-kernel - Re: [patch V3 17/18] posix-timers: Provide a mechanism to allocate a given timer ID

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: <Z9GFPffpN81va8bl@grain>
Date: Wed, 12 Mar 2025 15:59:41 +0300
From: Cyrill Gorcunov <gorcunov@...il.com>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	Anna-Maria Behnsen <anna-maria@...utronix.de>,
	Benjamin Segall <bsegall@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Andrey Vagin <avagin@...nvz.org>,
	Pavel Tikhomirov <ptikhomirov@...tuozzo.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [patch V3 17/18] posix-timers: Provide a mechanism to allocate a
 given timer ID

On Tue, Mar 11, 2025 at 10:35:46PM +0100, Frederic Weisbecker wrote:
> Le Sat, Mar 08, 2025 at 05:48:47PM +0100, Thomas Gleixner a écrit :
> > @@ -364,6 +389,16 @@ static enum hrtimer_restart posix_timer_
> >  	return HRTIMER_NORESTART;
> >  }
> >  
> > +long posixtimer_create_prctl(unsigned long ctrl)
> > +{
> > +	if (ctrl > PR_TIMER_CREATE_RESTORE_IDS_ON)
> > +		return -EINVAL;
> > +
> > +	guard(spinlock_irq)(&current->sighand->siglock);
> > +	current->signal->timer_create_restore_ids = ctrl == PR_TIMER_CREATE_RESTORE_IDS_ON;
> 
> Is the locking necessary here? It's not used on the read side.
> It only makes sense if more flags are to be added later in struct signal and the
> fields write can race.

Actually this is a very subtle moment. The @timer_create_restore_ids is a bit field and
updating them without a lock already lead into hard to catch bugs in the past especially
when we have close bits members such as is_child_subreaper/has_child_subreaper near it.
I thought of fork(clone_vm) calls in multithreaded application where real_parent may
point into our task which is doing prctl but didn't find any problem so far (though
internal feeling says that this is not hot path call and better would be to keep Thomas'
original lock code :-). Anyway, seems to be safe without it.

	Cyrill