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: <20160623161643.GA106079@f23x64.localdomain>
Date:	Thu, 23 Jun 2016 09:16:43 -0700
From:	Darren Hart <dvhart@...radead.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>,
	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 03:40:36PM +0200, Thomas Gleixner wrote:
> On Thu, 23 Jun 2016, Michael Kerrisk (man-pages) wrote:
> > On 06/23/2016 09:18 AM, Thomas Gleixner wrote:
> > Once upon a time, you told me the following:
> > 
> > On 15 May 2014 at 16:14, Thomas Gleixner <tglx@...utronix.de> wrote:
> > > On Thu, 15 May 2014, Michael Kerrisk (man-pages) wrote:
> > > > And that universe would love to have your documentation of
> > > > FUTEX_WAKE_BITSET and FUTEX_WAIT_BITSET ;-),
> > > 
> > > I give you almost the full treatment, but I leave REQUEUE_PI to Darren
> > > and FUTEX_WAKE_OP to Jakub. :)
> > > [...]
> > > FUTEX_CLOCK_REALTIME
> > > 
> > >         This option bit can be ored on the futex ops FUTEX_WAIT_BITSET
> > >         and FUTEX_WAIT_REQUEUE_PI
> > > 
> > >         If set the kernel treats the user space supplied timeout as
> > >         absolute time based on CLOCK_REALTIME.
> > > 
> > >         If not set the kernel treats the user space supplied timeout
> > >         as relative time.
> > Unfortunately, I should have checked the code more carefully...
> 
> Me too :)

Seems to be going around...

>  
> > Looking more carefully at the code, I see understand the situation
> > is the following:
> > 
> > FUTEX_LOCK_PI
> > 	Always uses CLOCK_REALTIME
> > 	'timeout' is absolute
> 
> Yes.
>  
> > FUTEX_WAIT_REQUEUE_PI
> > 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
> >         	determined by presence or absence of
> > 		FUTEX_CLOCK_REALTIME flag
> > 	'timeout' is absolute
> 
> Yes
> 
> > FUTEX_WAIT_BITSET
> > 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
> >         	determined by presence or absence of
> > 		FUTEX_CLOCK_REALTIME flag
> > 	'timeout' is absolute
> 
> Yes
>  
> > FUTEX_WAIT
> > 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
> >         	determined by presence or absence of
> > 		FUTEX_CLOCK_REALTIME flag
> > 	'timeout' is relative
> 
> Yes.
> 
> > I've amended the man page to describe those details.

OK, that confirms my question, timeout interpretation as relative or absolute is
based on the op code, not the CLOCK flag.

> > 
> > > The flag was explicitely added to allow FUTEX_WAIT to hand in absolute time.
> > 
> > When you say that the "flag was added", which flag do you mean? Or, did you
> > mean: "applying Matthieu's patch will allow FUTEX_WAIT to hand in absolute
> > time".
> 
> I didn't express myself clearly. When Darren added the support for
> CLOCK_REALTIME to FUTEX_WAIT I think he wanted to add absolute timeout
> support. Anything else does not make sense.

I sent that patch because reading the new man page it struck me as strange that
FUTEX_WAIT was restricted to CLOCK_MONOTONIC and the other op codes were not,
especially since FUTEX_WAIT is a just FUTEX_WAIT_BITSET with the mask set to
ALL.

I didn't realize the impact to relative/absolute interpretation of the timeout
value at the time.

I think it was a mistake to introduce a change that made FUTEX_WAIT interpret
the timeout differently based on the CLOCK flag, while that interpretation is
independent of the CLOCK flag for all other op codes.

In my opinion, we should treat the timeout value as relative for FUTEX_WAIT
regardless of the CLOCK used.

That would require a change to the man page to eliminate the relative/absolute
language in the FUTEX_CLOCK_REALTIME definition and explicit definitions of the
interpretation for each op code (as Matthew explains above).

Do we agree on that?

> 
> Thanks,
> 
> 	tglx
> 

-- 
Darren Hart
Intel Open Source Technology Center