lkml.org 
[lkml]   [2011]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex
On 08/16/2011 06:54 PM, Yong Zhang wrote:
> We can't trust in the caller(userspace) to give signed-extend
> parameter, thus futex-wait may fail in some special case.
>
> For example, if 'val' is too big and bit-31 is 1,
> the caller may enter endless loop at:
> futex_wait_setup()
> {
> ...
>
> if (uval != val) {
> queue_unlock(q, *hb);
> ret = -EWOULDBLOCK;
>
> ...
> }
>
> Below assembler code will make it more easy to understand how
> the patch take effect :)
>
> Dump of assembler code for function SyS_32_futex:
> 0xffffffff811b6fe8<+0>: sll a1,a1,0x0
> 0xffffffff811b6fec<+4>: sll a2,a2,0x0
> 0xffffffff811b6ff0<+8>: j 0xffffffff8121a240<compat_sys_futex>
> 0xffffffff811b6ff4<+12>: sll a5,a5,0x0
>
> Signed-off-by: Yong Zhang<yong.zhang@windriver.com>
> Cc: Ralf Baechle<ralf@linux-mips.org>
> ---
> arch/mips/kernel/linux32.c | 7 +++++++
> arch/mips/kernel/scall64-n32.S | 2 +-
> arch/mips/kernel/scall64-o32.S | 2 +-
> 3 files changed, 9 insertions(+), 2 deletions(-)
[...]
> diff --git a/arch/mips/kernel/linux32.c b/arch/mips/kernel/linux32.c
> index 876a75c..922a554 100644
> --- a/arch/mips/kernel/linux32.c
> +++ b/arch/mips/kernel/linux32.c
> @@ -349,3 +349,10 @@ SYSCALL_DEFINE6(32_fanotify_mark, int, fanotify_fd, unsigned int, flags,
> return sys_fanotify_mark(fanotify_fd, flags, merge_64(a3, a4),
> dfd, pathname);
> }
> +
> +SYSCALL_DEFINE6(32_futex, u32 __user *, uaddr, int, op, u32, val,
> + struct compat_timespec __user *, utime, u32 __user *, uaddr2,
> + u32, val3)
> +{
> + return compat_sys_futex(uaddr, op, val, utime, uaddr2, val3);
> +}
> diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
> index b85842f..c956cc9 100644
> --- a/arch/mips/kernel/scall64-n32.S
> +++ b/arch/mips/kernel/scall64-n32.S
> @@ -315,7 +315,7 @@ EXPORT(sysn32_call_table)
> PTR sys_fremovexattr
> PTR sys_tkill
> PTR sys_ni_syscall
> - PTR compat_sys_futex
> + PTR sys_32_futex
> PTR compat_sys_sched_setaffinity /* 6195 */
> PTR compat_sys_sched_getaffinity
> PTR sys_cacheflush
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index 46c4763..f48b18e 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -441,7 +441,7 @@ sys_call_table:
> PTR sys_fremovexattr /* 4235 */
> PTR sys_tkill
> PTR sys_sendfile64
> - PTR compat_sys_futex
> + PTR sys_32_futex

This change is redundant, scall64-o32.S already does the right thing
so additional zero extending is not needed and is just extra
instructions to execute for no reason.

> PTR compat_sys_sched_setaffinity
> PTR compat_sys_sched_getaffinity /* 4240 */
> PTR compat_sys_io_setup

But really I think this patch fixes things at the wrong level. Each
architecture potentially needs a similar patch. What would happen if
we did something like:


diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
index 5f9e689..74ada65 100644
--- a/kernel/futex_compat.c
+++ b/kernel/futex_compat.c
@@ -180,9 +180,9 @@ err_unlock:
return ret;
}

-asmlinkage long compat_sys_futex(u32 __user *uaddr, int op, u32 val,
- struct compat_timespec __user *utime, u32 __user *uaddr2,
- u32 val3)
+SYSCALL_DEFINE6(compat_sys_futex, u32 __user *, uaddr, int , op, u32, val,
+ struct compat_timespec __user *, utime, u32 __user *, uaddr2,
+ u32, val3)
{
struct timespec ts;
ktime_t t, *tp = NULL;
Obviously the function name is wrong, but a varient of
SYSCALL_DEFINE*() could be created so the proper function names are
produced.

David Daney


\
 
 \ /
  Last update: 2011-08-17 19:21    [W:0.070 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site