From: wei gao <[email protected]>
Current implementation lead LTP test case futex_waitv failed when compiled with
-m32. This patch add new compat_sys_futex_waitv to handle m32 mode syscall.
The failure reason is futex_waitv in m32 mode will deliver kernel with struct
old_timespec32 timeout, but this struct type can not directly used by current
sys_futex_waitv implementation.
The new function copy main logic of current sys_futex_waitv, just update parameter
type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
and use get_old_timespec32 within the new function to get timeout value.
Signed-off-by: wei gao <[email protected]>
---
arch/x86/entry/syscalls/syscall_32.tbl | 2 +-
kernel/futex/syscalls.c | 61 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index c8fac5205803..11bd927dd417 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -453,7 +453,7 @@
446 i386 landlock_restrict_self sys_landlock_restrict_self
447 i386 memfd_secret sys_memfd_secret
448 i386 process_mrelease sys_process_mrelease
-449 i386 futex_waitv sys_futex_waitv
+449 i386 futex_waitv sys_futex_waitv compat_sys_futex_waitv
450 i386 set_mempolicy_home_node sys_set_mempolicy_home_node
451 i386 cachestat sys_cachestat
452 i386 fchmodat2 sys_fchmodat2
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 4b6da9116aa6..62d69f8ec34c 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -486,6 +486,67 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
return ret;
}
+
+COMPAT_SYSCALL_DEFINE5(futex_waitv, struct futex_waitv __user *, waiters,
+ unsigned int, nr_futexes, unsigned int, flags,
+ struct old_timespec32 __user *, timeout, clockid_t, clockid)
+{
+ struct hrtimer_sleeper to;
+ struct futex_vector *futexv;
+ struct timespec64 ts;
+ ktime_t time;
+ int ret;
+
+ /* This syscall supports no flags for now */
+ if (flags)
+ return -EINVAL;
+
+ if (!nr_futexes || nr_futexes > FUTEX_WAITV_MAX || !waiters)
+ return -EINVAL;
+
+ if (timeout) {
+ int flag_clkid = 0, flag_init = 0;
+
+ if (clockid == CLOCK_REALTIME) {
+ flag_clkid = FLAGS_CLOCKRT;
+ flag_init = FUTEX_CLOCK_REALTIME;
+ }
+
+ if (clockid != CLOCK_REALTIME && clockid != CLOCK_MONOTONIC)
+ return -EINVAL;
+
+ if (get_old_timespec32(&ts, timeout))
+ return -EFAULT;
+
+ /*
+ * Since there's no opcode for futex_waitv, use
+ * FUTEX_WAIT_BITSET that uses absolute timeout as well
+ */
+ ret = futex_init_timeout(FUTEX_WAIT_BITSET, flag_init, &ts, &time);
+ if (ret)
+ return ret;
+
+ futex_setup_timer(&time, &to, flag_clkid, 0);
+ }
+
+ futexv = kcalloc(nr_futexes, sizeof(*futexv), GFP_KERNEL);
+ if (!futexv) {
+ ret = -ENOMEM;
+ goto destroy_timer;
+ }
+
+ ret = futex_parse_waitv(futexv, waiters, nr_futexes, futex_wake_mark,
+ NULL);
+ if (!ret)
+ ret = futex_wait_multiple(futexv, nr_futexes, timeout ? &to : NULL);
+
+ kfree(futexv);
+
+destroy_timer:
+ if (timeout)
+ futex2_destroy_timeout(&to);
+ return ret;
+}
#endif /* CONFIG_COMPAT */
#ifdef CONFIG_COMPAT_32BIT_TIME
--
2.34.1
[+CC Arnd]
Hi Wei,
Em 23/11/2023 02:31, Wei Gao escreveu:
> From: wei gao <[email protected]>
>
> Current implementation lead LTP test case futex_waitv failed when compiled with
> -m32. This patch add new compat_sys_futex_waitv to handle m32 mode syscall.
>
> The failure reason is futex_waitv in m32 mode will deliver kernel with struct
> old_timespec32 timeout, but this struct type can not directly used by current
> sys_futex_waitv implementation.
>
> The new function copy main logic of current sys_futex_waitv, just update parameter
> type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
> and use get_old_timespec32 within the new function to get timeout value.
>
From, what I recall, we don't want to add new syscalls with
old_timespec32, giving that they will have a limited lifetime. Instead,
userspace should be able to come up with a 64-bit timespec
implementation for -m32.
Thanks,
André
Hi Wei,
kernel test robot noticed the following build errors:
[auto build test ERROR on tip/x86/asm]
[also build test ERROR on tip/locking/core linus/master v6.7-rc2 next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Wei-Gao/futex-Add-compat_sys_futex_waitv-for-32bit-compatibility/20231123-133427
base: tip/x86/asm
patch link: https://lore.kernel.org/r/20231123053140.16062-1-wegao%40suse.com
patch subject: [PATCH v1] futex: Add compat_sys_futex_waitv for 32bit compatibility
config: x86_64-buildonly-randconfig-005-20231123 (https://download.01.org/0day-ci/archive/20231124/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> ld: arch/x86/entry/syscall_32.o:(.rodata+0xe08): undefined reference to `__ia32_compat_sys_futex_waitv'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Nov 23, 2023 at 01:09:55PM -0300, Andr? Almeida wrote:
> [+CC Arnd]
>
> Hi Wei,
>
> Em 23/11/2023 02:31, Wei Gao escreveu:
> > From: wei gao <[email protected]>
> >
> > Current implementation lead LTP test case futex_waitv failed when compiled with
> > -m32. This patch add new compat_sys_futex_waitv to handle m32 mode syscall.
> >
> > The failure reason is futex_waitv in m32 mode will deliver kernel with struct
> > old_timespec32 timeout, but this struct type can not directly used by current
> > sys_futex_waitv implementation.
> >
> > The new function copy main logic of current sys_futex_waitv, just update parameter
> > type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
> > and use get_old_timespec32 within the new function to get timeout value.
> >
>
> From, what I recall, we don't want to add new syscalls with old_timespec32,
> giving that they will have a limited lifetime. Instead, userspace should be
> able to come up with a 64-bit timespec implementation for -m32.
>
> Thanks,
> Andr?
Just a comment, I have checked the glibc latest code but do not see any implemention(*.c) on
futex_waitv syscall. So normally you have to do syscall directly with __NR_futex_waitv from
userspace. So i guess glibc-side can not covert this struct correctly currently. Correct me if
any misunderstanding.
Thanks
Wei Gao
Hi Wei,
Em 27/11/2023 09:15, Wei Gao escreveu:
> On Thu, Nov 23, 2023 at 01:09:55PM -0300, André Almeida wrote:
>> [+CC Arnd]
>>
>> Hi Wei,
>>
>> Em 23/11/2023 02:31, Wei Gao escreveu:
>>> From: wei gao <[email protected]>
>>>
>>> Current implementation lead LTP test case futex_waitv failed when compiled with
>>> -m32. This patch add new compat_sys_futex_waitv to handle m32 mode syscall.
>>>
>>> The failure reason is futex_waitv in m32 mode will deliver kernel with struct
>>> old_timespec32 timeout, but this struct type can not directly used by current
>>> sys_futex_waitv implementation.
>>>
>>> The new function copy main logic of current sys_futex_waitv, just update parameter
>>> type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
>>> and use get_old_timespec32 within the new function to get timeout value.
>>>
>>
>> From, what I recall, we don't want to add new syscalls with old_timespec32,
>> giving that they will have a limited lifetime. Instead, userspace should be
>> able to come up with a 64-bit timespec implementation for -m32.
>>
>> Thanks,
>> André
>
> Just a comment, I have checked the glibc latest code but do not see any implemention(*.c) on
> futex_waitv syscall. So normally you have to do syscall directly with __NR_futex_waitv from
> userspace. So i guess glibc-side can not covert this struct correctly currently. Correct me if
> any misunderstanding.
>
futex() has no syscall wrappers in glibc. Userspace needs to figure out
everything by themselves, including which struct they should use, and I
don't think that glibc does any conversion. If you create manually a
timespec64 that works in -m32, and pass this to sycall(__NR_futex_waitv,
..., &timeout, ...), it should work correctly. You can read more about
how glibc is planning to deal with this at [1]. Please let me know if
now it's more clear :)
[1] https://sourceware.org/glibc/wiki/Y2038ProofnessDesign
> Thanks
> Wei Gao
On Thu, Nov 23 2023 at 00:31, Wei Gao wrote:
> The new function copy main logic of current sys_futex_waitv, just update parameter
> type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
> and use get_old_timespec32 within the new function to get timeout
> value.
That's not how it works.
struct __kernel_timespec is the same on 64bit and 32bit syscalls.
User space has to use the proper type when invoking a syscall and and
not just decide that it can use something arbitrary.
All new syscalls which deal with time use the Y2038 aware data types and
do not have compat fallbacks because there is no requirement to have
them.
If user space want's to use struct timespec on 32bit nevertheless in the
programm for a new syscall, which is obviously stupid in the context of
Y2038, then it's a user space problem to convert back and forth between
the two data types.
Fix LTP to be Y2038 safe instead.
Thanks,
tglx
On Wed, Nov 29, 2023 at 03:56:12PM -0300, Andr? Almeida wrote:
> Hi Wei,
>
> Em 27/11/2023 09:15, Wei Gao escreveu:
> > On Thu, Nov 23, 2023 at 01:09:55PM -0300, Andr? Almeida wrote:
> > > [+CC Arnd]
> > >
> > > Hi Wei,
> > >
> > > Em 23/11/2023 02:31, Wei Gao escreveu:
> > > > From: wei gao <[email protected]>
> > > >
> > > > Current implementation lead LTP test case futex_waitv failed when compiled with
> > > > -m32. This patch add new compat_sys_futex_waitv to handle m32 mode syscall.
> > > >
> > > > The failure reason is futex_waitv in m32 mode will deliver kernel with struct
> > > > old_timespec32 timeout, but this struct type can not directly used by current
> > > > sys_futex_waitv implementation.
> > > >
> > > > The new function copy main logic of current sys_futex_waitv, just update parameter
> > > > type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
> > > > and use get_old_timespec32 within the new function to get timeout value.
> > > >
> > >
> > > From, what I recall, we don't want to add new syscalls with old_timespec32,
> > > giving that they will have a limited lifetime. Instead, userspace should be
> > > able to come up with a 64-bit timespec implementation for -m32.
> > >
> > > Thanks,
> > > Andr?
> >
> > Just a comment, I have checked the glibc latest code but do not see any implemention(*.c) on
> > futex_waitv syscall. So normally you have to do syscall directly with __NR_futex_waitv from
> > userspace. So i guess glibc-side can not covert this struct correctly currently. Correct me if
> > any misunderstanding.
> >
>
> futex() has no syscall wrappers in glibc. Userspace needs to figure out
> everything by themselves, including which struct they should use, and I
> don't think that glibc does any conversion. If you create manually a
> timespec64 that works in -m32, and pass this to sycall(__NR_futex_waitv,
> ..., &timeout, ...), it should work correctly. You can read more about how
> glibc is planning to deal with this at [1]. Please let me know if now it's
> more clear :)
>
> [1] https://sourceware.org/glibc/wiki/Y2038ProofnessDesign
Thanks a lot for your detail explaination and good learning link, it's more clear to me now : )
>
> > Thanks
> > Wei Gao
On Wed, Nov 29, 2023 at 09:54:56PM +0100, Thomas Gleixner wrote:
> On Thu, Nov 23 2023 at 00:31, Wei Gao wrote:
> > The new function copy main logic of current sys_futex_waitv, just update parameter
> > type from "struct __kernel_timespec __user *" to "struct old_timespec32 __user *,"
> > and use get_old_timespec32 within the new function to get timeout
> > value.
>
> That's not how it works.
>
> struct __kernel_timespec is the same on 64bit and 32bit syscalls.
>
> User space has to use the proper type when invoking a syscall and and
> not just decide that it can use something arbitrary.
>
> All new syscalls which deal with time use the Y2038 aware data types and
> do not have compat fallbacks because there is no requirement to have
> them.
>
> If user space want's to use struct timespec on 32bit nevertheless in the
> programm for a new syscall, which is obviously stupid in the context of
> Y2038, then it's a user space problem to convert back and forth between
> the two data types.
>
> Fix LTP to be Y2038 safe instead.
Thanks a lot for your suggestion, will check it.
>
> Thanks,
>
> tglx