linux-kernel - Re: [patch V2 10/50] posix-cpu-timers: Handle SIGEV_NONE timers correctly in timer

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: <87h6g6z7hs.ffs@tglx>
Date: Fri, 12 Apr 2024 21:49:35 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Anna-Maria Behnsen
 <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>,
 John Stultz <jstultz@...gle.com>, Peter Zijlstra <peterz@...radead.org>,
 Ingo Molnar <mingo@...nel.org>, Stephen Boyd <sboyd@...nel.org>, Oleg
 Nesterov <oleg@...hat.com>
Subject: Re: [patch V2 10/50] posix-cpu-timers: Handle SIGEV_NONE timers
 correctly in timer_get()

On Fri, Apr 12 2024 at 13:40, Eric W. Biederman wrote:
> Thomas Gleixner <tglx@...utronix.de> writes:
>> -	if (iv && (timer->it_requeue_pending & REQUEUE_PENDING))
>> +	if (iv && ((timer->it_requeue_pending & REQUEUE_PENDING) || sigev_none))
>>  		expires = bump_cpu_timer(timer, now);
>>  	else
>>  		expires = cpu_timer_getexpires(&timer->it.cpu);
>> @@ -809,11 +811,13 @@ static void __posix_cpu_timer_get(struct
>>  		itp->it_value = ns_to_timespec64(expires - now);
>>  	} else {
> Why not make this else condition?
> 	} else if (!sigev_none) {
> And not need to change the rest of the code?

Duh, yes.
  		/*
>> -		 * The timer should have expired already, but the firing
>> -		 * hasn't taken place yet.  Say it's just about to expire.
>> +		 * A single shot SIGEV_NONE timer must return 0, when it is
>> +		 * expired! Timers which have a real signal delivery mode
>> +		 * must return a remaining time greater than 0 because the
>> +		 * signal has not yet been delivered.
>>  		 */
>> -		itp->it_value.tv_nsec = 1;
>> -		itp->it_value.tv_sec = 0;
>> +		if (!sigev_none)
>> +			itp->it_value.tv_nsec = 1;
>
> Do you perhaps need a comment somewhere that itp is zeroed in
> do_timer_gettime?  The code now depends upon that for setting
> itp->it_value when it did not used to, making it look at first
> glance like you have created an uninitialized variable.
>
> Probably just something in the description of the change would be
> sufficient.

Fair enough.

Thanks,

        tglx