On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote: > Cause syscall handlers to be typed as follows when called indirectly > throughout the kernel. > > typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long, > unsigned long, unsigned long, unsigned long);
The point is... better type checking? > > Since both 32 and 64-bit abis allow for at least the first six > machine-word length parameters to a function to be passed by registers, > even handlers which admit fewer than six parameters may be viewed as > having the above type. > > Fixup comparisons in VDSO to avoid pointer-integer comparison. Introduce > explicit cast on systems with SPUs. > > Signed-off-by: Rohan McLure <rmcl...@linux.ibm.com> > --- > V1 -> V2: New patch. > V2 -> V3: Remove unnecessary cast from const syscall_fn to syscall_fn > --- > arch/powerpc/include/asm/syscall.h | 7 +++++-- > arch/powerpc/include/asm/syscalls.h | 1 + > arch/powerpc/kernel/systbl.c | 6 +++--- > arch/powerpc/kernel/vdso.c | 4 ++-- > arch/powerpc/platforms/cell/spu_callbacks.c | 6 +++--- > 5 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/include/asm/syscall.h > b/arch/powerpc/include/asm/syscall.h > index 25fc8ad9a27a..d2a8dfd5de33 100644 > --- a/arch/powerpc/include/asm/syscall.h > +++ b/arch/powerpc/include/asm/syscall.h > @@ -14,9 +14,12 @@ > #include <linux/sched.h> > #include <linux/thread_info.h> > > +typedef long (*syscall_fn)(unsigned long, unsigned long, unsigned long, > + unsigned long, unsigned long, unsigned long); > + > /* ftrace syscalls requires exporting the sys_call_table */ > -extern const unsigned long sys_call_table[]; > -extern const unsigned long compat_sys_call_table[]; > +extern const syscall_fn sys_call_table[]; > +extern const syscall_fn compat_sys_call_table[]; Ah you constify it in this patch. I think the previous patch should have kept the const, and it should keep the unsigned long type rather than use void *. Either that or do this patch first. > static inline int syscall_get_nr(struct task_struct *task, struct pt_regs > *regs) > { > diff --git a/arch/powerpc/include/asm/syscalls.h > b/arch/powerpc/include/asm/syscalls.h > index 91417dee534e..e979b7593d2b 100644 > --- a/arch/powerpc/include/asm/syscalls.h > +++ b/arch/powerpc/include/asm/syscalls.h > @@ -8,6 +8,7 @@ > #include <linux/types.h> > #include <linux/compat.h> > > +#include <asm/syscall.h> > #ifdef CONFIG_PPC64 > #include <asm/syscalls_32.h> > #endif Is this necessary or should be in another patch? > diff --git a/arch/powerpc/kernel/systbl.c b/arch/powerpc/kernel/systbl.c > index 99ffdfef6b9c..b88a9c2a1f50 100644 > --- a/arch/powerpc/kernel/systbl.c > +++ b/arch/powerpc/kernel/systbl.c > @@ -21,10 +21,10 @@ > #define __SYSCALL(nr, entry) [nr] = __powerpc_##entry, > #define __powerpc_sys_ni_syscall sys_ni_syscall > #else > -#define __SYSCALL(nr, entry) [nr] = entry, > +#define __SYSCALL(nr, entry) [nr] = (void *) entry, > #endif Also perhaps this should have been in the prior pach and this pach should change the cast from void to syscall_fn ? > > -void *sys_call_table[] = { > +const syscall_fn sys_call_table[] = { > #ifdef CONFIG_PPC64 > #include <asm/syscall_table_64.h> > #else > @@ -35,7 +35,7 @@ void *sys_call_table[] = { > #ifdef CONFIG_COMPAT > #undef __SYSCALL_WITH_COMPAT > #define __SYSCALL_WITH_COMPAT(nr, native, compat) __SYSCALL(nr, compat) > -void *compat_sys_call_table[] = { > +const syscall_fn compat_sys_call_table[] = { > #include <asm/syscall_table_32.h> > }; > #endif /* CONFIG_COMPAT */ > diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c > index 0da287544054..080c9e7de0c0 100644 > --- a/arch/powerpc/kernel/vdso.c > +++ b/arch/powerpc/kernel/vdso.c > @@ -313,10 +313,10 @@ static void __init vdso_setup_syscall_map(void) > unsigned int i; > > for (i = 0; i < NR_syscalls; i++) { > - if (sys_call_table[i] != (unsigned long)&sys_ni_syscall) > + if (sys_call_table[i] != (void *)&sys_ni_syscall) > vdso_data->syscall_map[i >> 5] |= 0x80000000UL >> (i & > 0x1f); > if (IS_ENABLED(CONFIG_COMPAT) && > - compat_sys_call_table[i] != (unsigned long)&sys_ni_syscall) > + compat_sys_call_table[i] != (void *)&sys_ni_syscall) Also these. Thanks, Nick > vdso_data->compat_syscall_map[i >> 5] |= 0x80000000UL > >> (i & 0x1f); > } > } > diff --git a/arch/powerpc/platforms/cell/spu_callbacks.c > b/arch/powerpc/platforms/cell/spu_callbacks.c > index fe0d8797a00a..e780c14c5733 100644 > --- a/arch/powerpc/platforms/cell/spu_callbacks.c > +++ b/arch/powerpc/platforms/cell/spu_callbacks.c > @@ -34,15 +34,15 @@ > * mbind, mq_open, ipc, ... > */ > > -static void *spu_syscall_table[] = { > +static const syscall_fn spu_syscall_table[] = { > #define __SYSCALL_WITH_COMPAT(nr, entry, compat) __SYSCALL(nr, entry) > -#define __SYSCALL(nr, entry) [nr] = entry, > +#define __SYSCALL(nr, entry) [nr] = (void *) entry, > #include <asm/syscall_table_spu.h> > }; > > long spu_sys_callback(struct spu_syscall_block *s) > { > - long (*syscall)(u64 a1, u64 a2, u64 a3, u64 a4, u64 a5, u64 a6); > + syscall_fn syscall; > > if (s->nr_ret >= ARRAY_SIZE(spu_syscall_table)) { > pr_debug("%s: invalid syscall #%lld", __func__, s->nr_ret); > -- > 2.34.1