linux-kernel - Re: futex: Allow FUTEX_CLOCK_REALTIME with FUTEX

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: <20160623195515.GD106079@f23x64.localdomain>
Date:	Thu, 23 Jun 2016 12:55:15 -0700
From:	Darren Hart <dvhart@...radead.org>
To:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Matthieu CASTET <matthieu.castet@...rot.com>,
	linux-kernel@...r.kernel.org, Darren Hart <dvhart@...ux.intel.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Eric Dumazet <dada1@...mosbay.com>
Subject: Re: futex: Allow FUTEX_CLOCK_REALTIME with FUTEX_WAIT op

On Thu, Jun 23, 2016 at 08:41:09PM +0200, Michael Kerrisk (man-pages) wrote:
> On 06/23/2016 08:28 PM, Darren Hart wrote:
> > On Thu, Jun 23, 2016 at 07:26:52PM +0200, Thomas Gleixner wrote:
> > > On Thu, 23 Jun 2016, Darren Hart wrote:
> > > > On Thu, Jun 23, 2016 at 03:40:36PM +0200, Thomas Gleixner wrote:
> > > > In my opinion, we should treat the timeout value as relative for FUTEX_WAIT
> > > > regardless of the CLOCK used.
> > > 
> > > Which requires even more changes as you have to select which clock you are
> > > using for adding the base time.
> > 
> > Right, something like the following?
> > 
> > 
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index 33664f7..c39d807 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -3230,8 +3230,12 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> >  			return -EINVAL;
> > 
> >  		t = timespec_to_ktime(ts);
> > -		if (cmd == FUTEX_WAIT)
> > -			t = ktime_add_safe(ktime_get(), t);
> > +		if (cmd == FUTEX_WAIT) {
> > +			if (cmd & FUTEX_CLOCK_REALTIME)
> > +				t = ktime_add_safe(ktime_get_real(), t);
> > +			else
> > +				t = ktime_add_safe(ktime_get(), t);
> > +		}
> >  		tp = &t;
> >  	}
> >  	/*
> 
> Just in the interests of readability/maintainability, might it not
> make some sense to recode the timeout handling for FUTEX_WAIT
> within futex_wait(). I think that part of the reason we're in this
> mess of inconsistency is that timeout interpretation is being handled
> at too many different points in the code.


I agree, that is indeed why I missed it in my original patch.


> 
> > And as a follow-on, what is the reason for FUTEX_LOCK_PI only using
> > CLOCK_REALTIME? It seems reasonable to me that a user may want to wait a
> > specific amount of time, regardless of wall time.
> 
> Yes, that's another weird inconsistency.
> 
> Thanks,
> 
> Michael
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/
> 

-- 
Darren Hart
Intel Open Source Technology Center