* [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex
@ 2011-08-17 1:54 Yong Zhang
2011-08-17 12:43 ` Ralf Baechle
2011-08-17 17:17 ` David Daney
0 siblings, 2 replies; 10+ messages in thread
From: Yong Zhang @ 2011-08-17 1:54 UTC (permalink / raw)
To: linux-mips, linux-kernel; +Cc: Ralf Baechle
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
PTR compat_sys_sched_setaffinity
PTR compat_sys_sched_getaffinity /* 4240 */
PTR compat_sys_io_setup
--
1.7.4.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-17 1:54 [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex Yong Zhang @ 2011-08-17 12:43 ` Ralf Baechle 2011-08-17 17:17 ` David Daney 1 sibling, 0 replies; 10+ messages in thread From: Ralf Baechle @ 2011-08-17 12:43 UTC (permalink / raw) To: Yong Zhang; +Cc: linux-mips, linux-kernel Applied. Thanks Yong! Ralf ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-17 1:54 [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex Yong Zhang 2011-08-17 12:43 ` Ralf Baechle @ 2011-08-17 17:17 ` David Daney 2011-08-18 2:32 ` Yong Zhang ` (2 more replies) 1 sibling, 3 replies; 10+ messages in thread From: David Daney @ 2011-08-17 17:17 UTC (permalink / raw) To: Yong Zhang; +Cc: linux-mips, linux-kernel, Ralf Baechle 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-17 17:17 ` David Daney @ 2011-08-18 2:32 ` Yong Zhang 2011-08-18 16:23 ` David Daney 2011-08-18 2:44 ` Yong Zhang 2011-08-18 20:19 ` Ralf Baechle 2 siblings, 1 reply; 10+ messages in thread From: Yong Zhang @ 2011-08-18 2:32 UTC (permalink / raw) To: David Daney; +Cc: linux-mips, linux-kernel, Ralf Baechle On Wed, Aug 17, 2011 at 10:17:52AM -0700, David Daney wrote: > >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 My first virsion(not sent out) doesn't include scall64-o32.S either. > so additional zero extending is not needed and is just extra > instructions to execute for no reason. Why I'm adding it here is for: 1)code consistent, otherwise we must move SYSCALL_DEFINE6(32_futex,...) under CONFIG_MIPS32_N32; 2)I'm afraid there may be some other way to touch the high 32-bit of a register, so touching scall64-o32.S is also for safety(due to unknown reason, fix me if I'm wrong). Thanks, Yong ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-18 2:32 ` Yong Zhang @ 2011-08-18 16:23 ` David Daney 2011-08-19 1:56 ` Yong Zhang 0 siblings, 1 reply; 10+ messages in thread From: David Daney @ 2011-08-18 16:23 UTC (permalink / raw) To: Yong Zhang; +Cc: linux-mips, linux-kernel, Ralf Baechle On 08/17/2011 07:32 PM, Yong Zhang wrote: > On Wed, Aug 17, 2011 at 10:17:52AM -0700, David Daney wrote: >>> 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 > > My first virsion(not sent out) doesn't include scall64-o32.S either. > >> so additional zero extending is not needed and is just extra >> instructions to execute for no reason. > > Why I'm adding it here is for: > 1)code consistent, otherwise we must move SYSCALL_DEFINE6(32_futex,...) > under CONFIG_MIPS32_N32; No, you don't have to move it. Just don't call it. > 2)I'm afraid there may be some other way to touch the high 32-bit of a > register, so touching scall64-o32.S is also for safety(due to unknown > reason, fix me if I'm wrong). OK: You are mistaken. You claim you don't understand what the code does. That is really a poor justification for modifying it. David Daney ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-18 16:23 ` David Daney @ 2011-08-19 1:56 ` Yong Zhang 0 siblings, 0 replies; 10+ messages in thread From: Yong Zhang @ 2011-08-19 1:56 UTC (permalink / raw) To: David Daney; +Cc: linux-mips, linux-kernel, Ralf Baechle On Thu, Aug 18, 2011 at 09:23:41AM -0700, David Daney wrote: > On 08/17/2011 07:32 PM, Yong Zhang wrote: > >On Wed, Aug 17, 2011 at 10:17:52AM -0700, David Daney wrote: > >>>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 > > > >My first virsion(not sent out) doesn't include scall64-o32.S either. > > > >>so additional zero extending is not needed and is just extra > >>instructions to execute for no reason. > > > >Why I'm adding it here is for: > >1)code consistent, otherwise we must move SYSCALL_DEFINE6(32_futex,...) > > under CONFIG_MIPS32_N32; > > No, you don't have to move it. Just don't call it. > > > >2)I'm afraid there may be some other way to touch the high 32-bit of a > > register, so touching scall64-o32.S is also for safety(due to unknown > > reason, fix me if I'm wrong). > > OK: You are mistaken. You claim you don't understand what the code > does. That is really a poor justification for modifying it. If you don't like it and you are sure there is no potential security problem, just make a patch to remove it. Go ahead. Thanks, Yong ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-17 17:17 ` David Daney 2011-08-18 2:32 ` Yong Zhang @ 2011-08-18 2:44 ` Yong Zhang 2011-08-18 20:19 ` Ralf Baechle 2 siblings, 0 replies; 10+ messages in thread From: Yong Zhang @ 2011-08-18 2:44 UTC (permalink / raw) To: David Daney; +Cc: linux-mips, linux-kernel, Ralf Baechle On Wed, Aug 17, 2011 at 10:17:52AM -0700, David Daney wrote: > > 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: I have thought about it but finally I decide to keep it under arch code; because sparc64 and s390 have the same issue and they all smooth the concern by themselves. For sparc64: arch/sparc/kernel/sys32.S: 91: SIGN3(sys32_futex, compat_sys_futex, %o1, %o2, %o5) For s390: arch/s390/kernel/compat_wrapper.S ENTRY(compat_sys_futex_wrapper) llgtr %r2,%r2 # u32 * lgfr %r3,%r3 # int lgfr %r4,%r4 # int llgtr %r5,%r5 # struct compat_timespec * llgtr %r6,%r6 # u32 * lgf %r0,164(%r15) # int stg %r0,160(%r15) jg compat_sys_futex # branch to system call Maybe we can consolidate all of this like your patch in the future, but it's a different issue. Thanks, Yong > > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex 2011-08-17 17:17 ` David Daney 2011-08-18 2:32 ` Yong Zhang 2011-08-18 2:44 ` Yong Zhang @ 2011-08-18 20:19 ` Ralf Baechle 2011-08-19 3:49 ` How to trace compat syscalls? [Was Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex] Yong Zhang 2 siblings, 1 reply; 10+ messages in thread From: Ralf Baechle @ 2011-08-18 20:19 UTC (permalink / raw) To: David Daney; +Cc: Yong Zhang, linux-mips, linux-kernel On Wed, Aug 17, 2011 at 10:17:52AM -0700, David Daney wrote: > >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. Compat_sys_futex is a syscall entry point and for some configurations such as CONFIG_FTRACE_SYSCALLS SYSCALL_DEFINE*() will do additional work beyond cleaning up the arguments. The 3 unnecessary shifts are the overhead we just gotta live with. > > 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: > +++ 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. Right now none of the the generic compat_ functions is wrapped in SYSCALL_DEFINE* because for some architectures a further wrapper function is needed. It seems some architectures call compat_ calls directly without SYSCALL_DEFINE* which with CONFIG_FTRACE_SYSCALLS is a bug ... Ralf ^ permalink raw reply [flat|nested] 10+ messages in thread
* How to trace compat syscalls? [Was Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex] 2011-08-18 20:19 ` Ralf Baechle @ 2011-08-19 3:49 ` Yong Zhang 2011-08-19 4:15 ` Yong Zhang 0 siblings, 1 reply; 10+ messages in thread From: Yong Zhang @ 2011-08-19 3:49 UTC (permalink / raw) To: Ralf Baechle Cc: David Daney, linux-mips, linux-kernel, linux-arch, Yong Zhang, Steven Rostedt, Frederic Weisbecker, Ingo Molnar, Martin Schwidefsky, David S. Miller, Thomas Gleixner, Andrew Morton Cc'ing more people. On Thu, Aug 18, 2011 at 09:19:11PM +0100, Ralf Baechle wrote: > > 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: > > > +++ 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. > > Right now none of the the generic compat_ functions is wrapped in > SYSCALL_DEFINE* because for some architectures a further wrapper function > is needed. It seems some architectures call compat_ calls directly > without SYSCALL_DEFINE* which with CONFIG_FTRACE_SYSCALLS is a bug ... Just checked some archs which have HAVE_SYSCALL_TRACEPOINTS=y and could call compat_* syscalls when run 32bit process on 64bit kernel, I don't find any special code against FTRACE_SYSCALLS. So that means we could not trace compat syscalls even if we want to(Am I missing something?). So I think if we want to trace it, the easy way is just like what we have done on normal syscalls, IOW, we could SYSCALL_DEFINE all of the compat syscalls. Thought? BTW, I have make a trival patch to introduce COMPAT_SYSCALL_DEFINE, it's just for calling of inspiration(no test no build, and I leave SYSCALL_METADATA etc untouched.) Thanks, Yong --- diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 8c03b98..e79027f 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -221,26 +221,38 @@ extern struct trace_event_functions exit_syscall_print_funcs; __SC_STR_ADECL##x(__VA_ARGS__) \ }; \ SYSCALL_METADATA(sname, x); \ - __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) + __SYSCALL_DEFINEx(, x, sname, __VA_ARGS__) + +#define COMPAT_SYSCALL_DEFINEx(x, sname, ...) \ + static const char *types_compat_##sname[] = { \ + __SC_STR_TDECL##x(__VA_ARGS__) \ + }; \ + static const char *args_compat_##sname[] = { \ + __SC_STR_ADECL##x(__VA_ARGS__) \ + }; \ + SYSCALL_METADATA(compat, sname, x); \ + __SYSCALL_DEFINEx(cmopat_, x, sname, __VA_ARGS__) #else #define SYSCALL_DEFINEx(x, sname, ...) \ - __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) + __SYSCALL_DEFINEx(, x, sname, __VA_ARGS__) +#define COMPAT_SYSCALL_DEFINEx(x, sname, ...) \ + __SYSCALL_DEFINEx(compat_, x, sname, __VA_ARGS__) #endif #ifdef CONFIG_HAVE_SYSCALL_WRAPPERS -#define SYSCALL_DEFINE(name) static inline long SYSC_##name +#define SYSCALL_DEFINE(compat, name) static inline long compat##SYSC_##name -#define __SYSCALL_DEFINEx(x, name, ...) \ - asmlinkage long sys##name(__SC_DECL##x(__VA_ARGS__)); \ - static inline long SYSC##name(__SC_DECL##x(__VA_ARGS__)); \ - asmlinkage long SyS##name(__SC_LONG##x(__VA_ARGS__)) \ +#define __SYSCALL_DEFINEx(compat, x, name, ...) \ + asmlinkage long compat##sys##name(__SC_DECL##x(__VA_ARGS__)); \ + static inline long compat##SYSC##name(__SC_DECL##x(__VA_ARGS__));\ + asmlinkage long compat##SyS##name(__SC_LONG##x(__VA_ARGS__)) \ { \ __SC_TEST##x(__VA_ARGS__); \ - return (long) SYSC##name(__SC_CAST##x(__VA_ARGS__)); \ + return (long) compat##SYSC##name(__SC_CAST##x(__VA_ARGS__));\ } \ - SYSCALL_ALIAS(sys##name, SyS##name); \ - static inline long SYSC##name(__SC_DECL##x(__VA_ARGS__)) + SYSCALL_ALIAS(compat##sys##name, compat##SyS##name); \ + static inline long compat##SYSC##name(__SC_DECL##x(__VA_ARGS__)) #else /* CONFIG_HAVE_SYSCALL_WRAPPERS */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: How to trace compat syscalls? [Was Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex] 2011-08-19 3:49 ` How to trace compat syscalls? [Was Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex] Yong Zhang @ 2011-08-19 4:15 ` Yong Zhang 0 siblings, 0 replies; 10+ messages in thread From: Yong Zhang @ 2011-08-19 4:15 UTC (permalink / raw) To: Ralf Baechle Cc: David Daney, linux-mips, linux-kernel, linux-arch, Yong Zhang, Steven Rostedt, Frederic Weisbecker, Ingo Molnar, Martin Schwidefsky, David S. Miller, Thomas Gleixner, Andrew Morton On Fri, Aug 19, 2011 at 11:49:50AM +0800, Yong Zhang wrote: > Cc'ing more people. > > On Thu, Aug 18, 2011 at 09:19:11PM +0100, Ralf Baechle wrote: > > > 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: > > > > > +++ 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. > > > > Right now none of the the generic compat_ functions is wrapped in > > SYSCALL_DEFINE* because for some architectures a further wrapper function > > is needed. It seems some architectures call compat_ calls directly > > without SYSCALL_DEFINE* which with CONFIG_FTRACE_SYSCALLS is a bug ... > > Just checked some archs which have HAVE_SYSCALL_TRACEPOINTS=y and could > call compat_* syscalls when run 32bit process on 64bit kernel, I don't > find any special code against FTRACE_SYSCALLS. So that means we could > not trace compat syscalls even if we want to(Am I missing something?). > So I think if we want to trace it, the easy way is just like what we have > done on normal syscalls, IOW, we could SYSCALL_DEFINE all of the compat > syscalls. Except the ones which call sys_* finally, :) > > Thought? > > BTW, I have make a trival patch to introduce COMPAT_SYSCALL_DEFINE, it's just > for calling of inspiration(no test no build, and I leave SYSCALL_METADATA > etc untouched.) > > Thanks, > Yong > > --- > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index 8c03b98..e79027f 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -221,26 +221,38 @@ extern struct trace_event_functions exit_syscall_print_funcs; > __SC_STR_ADECL##x(__VA_ARGS__) \ > }; \ > SYSCALL_METADATA(sname, x); \ > - __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > + __SYSCALL_DEFINEx(, x, sname, __VA_ARGS__) > + > +#define COMPAT_SYSCALL_DEFINEx(x, sname, ...) \ > + static const char *types_compat_##sname[] = { \ > + __SC_STR_TDECL##x(__VA_ARGS__) \ > + }; \ > + static const char *args_compat_##sname[] = { \ > + __SC_STR_ADECL##x(__VA_ARGS__) \ > + }; \ > + SYSCALL_METADATA(compat, sname, x); \ > + __SYSCALL_DEFINEx(cmopat_, x, sname, __VA_ARGS__) > #else > #define SYSCALL_DEFINEx(x, sname, ...) \ > - __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > + __SYSCALL_DEFINEx(, x, sname, __VA_ARGS__) > +#define COMPAT_SYSCALL_DEFINEx(x, sname, ...) \ > + __SYSCALL_DEFINEx(compat_, x, sname, __VA_ARGS__) > #endif > > #ifdef CONFIG_HAVE_SYSCALL_WRAPPERS > > -#define SYSCALL_DEFINE(name) static inline long SYSC_##name > +#define SYSCALL_DEFINE(compat, name) static inline long compat##SYSC_##name > > -#define __SYSCALL_DEFINEx(x, name, ...) \ > - asmlinkage long sys##name(__SC_DECL##x(__VA_ARGS__)); \ > - static inline long SYSC##name(__SC_DECL##x(__VA_ARGS__)); \ > - asmlinkage long SyS##name(__SC_LONG##x(__VA_ARGS__)) \ > +#define __SYSCALL_DEFINEx(compat, x, name, ...) \ > + asmlinkage long compat##sys##name(__SC_DECL##x(__VA_ARGS__)); \ > + static inline long compat##SYSC##name(__SC_DECL##x(__VA_ARGS__));\ > + asmlinkage long compat##SyS##name(__SC_LONG##x(__VA_ARGS__)) \ > { \ > __SC_TEST##x(__VA_ARGS__); \ > - return (long) SYSC##name(__SC_CAST##x(__VA_ARGS__)); \ > + return (long) compat##SYSC##name(__SC_CAST##x(__VA_ARGS__));\ > } \ > - SYSCALL_ALIAS(sys##name, SyS##name); \ > - static inline long SYSC##name(__SC_DECL##x(__VA_ARGS__)) > + SYSCALL_ALIAS(compat##sys##name, compat##SyS##name); \ > + static inline long compat##SYSC##name(__SC_DECL##x(__VA_ARGS__)) > > #else /* CONFIG_HAVE_SYSCALL_WRAPPERS */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-08-19 4:16 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-08-17 1:54 [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex Yong Zhang 2011-08-17 12:43 ` Ralf Baechle 2011-08-17 17:17 ` David Daney 2011-08-18 2:32 ` Yong Zhang 2011-08-18 16:23 ` David Daney 2011-08-19 1:56 ` Yong Zhang 2011-08-18 2:44 ` Yong Zhang 2011-08-18 20:19 ` Ralf Baechle 2011-08-19 3:49 ` How to trace compat syscalls? [Was Re: [PATCH] MIPS: use 32-bit wrapper for compat_sys_futex] Yong Zhang 2011-08-19 4:15 ` Yong Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox