linux-kernel - Re: [PATCH v3] selftests/futex: Convert 32bit timespec struct to 64bit version for 32bit compatibility mode

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] [day] [month] [year] [list]

Message-ID: <aGcZDjYh6OLWRFkc@MiWiFi-CR6608-srv>
Date: Thu, 3 Jul 2025 19:58:06 -0400
From: Wei Gao <wegao@...e.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Terry Tritton <terry.tritton@...aro.org>, Shuah Khan <shuah@...nel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Darren Hart <dvhart@...radead.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	André Almeida <andrealmeid@...lia.com>,
	ttritton@...gle.com, linux-kselftest@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] selftests/futex: Convert 32bit timespec struct to
 64bit version for 32bit compatibility mode

On Wed, Jul 02, 2025 at 10:15:06PM +0200, Thomas Gleixner wrote:
> On Wed, Jul 02 2025 at 11:21, Terry Tritton wrote:
> 
> > Futex_waitv can not accept old_timespec32 struct, so userspace should
> 
> sys_futex_wait()
> 
> > convert it from 32bit to 64bit before syscall in 32bit compatible mode.
> >
> > This fix is based off [1]
> >
> > Link: https://lore.kernel.org/all/20231203235117.29677-1-wegao@suse.com/ [1]
> >
> > Originally-by: Wei Gao <wegao@...e.com>
> > Signed-off-by: Terry Tritton <terry.tritton@...aro.org>
> > ---
> > Changes in v3:
> > - Fix signed-off-by chain but for real this time
> >
> > Changes in v2:
> > - Fix signed-off-by chain
> >
> >  .../testing/selftests/futex/include/futex2test.h  | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/tools/testing/selftests/futex/include/futex2test.h b/tools/testing/selftests/futex/include/futex2test.h
> > index ea79662405bc..6780e51eb2d6 100644
> > --- a/tools/testing/selftests/futex/include/futex2test.h
> > +++ b/tools/testing/selftests/futex/include/futex2test.h
> > @@ -55,6 +55,13 @@ struct futex32_numa {
> >  	futex_t numa;
> >  };
> >  
> > +#if !defined(__LP64__)
> > +struct timespec64 {
> > +	int64_t tv_sec;
> > +	int64_t tv_nsec;
> > +};
> > +#endif
> 
> Why not use __kernel_timespec which is the actual data type that syscall
> requests?
> 
> > +
> >  /**
> >   * futex_waitv - Wait at multiple futexes, wake on any
> >   * @waiters:    Array of waiters
> > @@ -65,7 +72,15 @@ struct futex32_numa {
> >  static inline int futex_waitv(volatile struct futex_waitv *waiters, unsigned long nr_waiters,
> >  			      unsigned long flags, struct timespec *timo, clockid_t clockid)
> >  {
> > +#if !defined(__LP64__)
> > +	struct timespec64 timo64 = {0};
> > +
> > +	timo64.tv_sec = timo->tv_sec;
> > +	timo64.tv_nsec = timo->tv_nsec;
> > +	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, &timo64, clockid);
> > +#else
> >  	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, timo, clockid);
> > +#endif
> 
> You simply can do
> 
>         struct __kernel_timespec ts = {
>         	.tv_sec = timo->tv_sec,
>                 .tv_nsec = timo->tv_nsec,
>         };
> 
>   	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, &ts, clockid);
>         
> unconditionally. No?
Thanks for your valuable comments.
Your change has more directly interacts with system calls and simplifies the code without
conditional compilation. 
Only concern from my side is test on very old kernel(But i think this is not problem for kself test)
> 
> Thanks,
> 
>         tglx