On Fri, Oct 27, 2023 at 09:36:25AM -0600, Theo de Raadt wrote:
> Index: sys/arch/m88k/m88k/trap.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/m88k/m88k/trap.c,v
> diff -u -p -u -r1.128 trap.c
> --- sys/arch/m88k/m88k/trap.c 2 Aug 2023 06:14:46 -0000 1.128
> +++ sys/arch/m88k/m88k/trap.c 27 Oct 2023 03:17:47 -0000
> @@ -1153,9 +1153,9 @@ void
> m88100_syscall(register_t code, struct trapframe *tf)
> {
> int i, nap;
> - const struct sysent *callp;
> + const struct sysent *callp = sysent;
> struct proc *p = curproc;
> - int error, indirect = -1;
> + int error;
> register_t args[8] __aligned(8);
> register_t rval[2] __aligned(8);
> register_t *ap;
> @@ -1172,19 +1172,9 @@ m88100_syscall(register_t code, struct t
> ap = &tf->tf_r[2];
> nap = 8; /* r2-r9 */
>
> - switch (code) {
> - case SYS_syscall:
> - indirect = code;
> - code = *ap++;
> - nap--;
> - break;
> - }
> -
> - callp = sysent;
> - if (code < 0 || code >= SYS_MAXSYSCALL)
> - callp += SYS_syscall;
> - else
> - callp += code;
> + if (code <= 0 || code >= SYS_MAXSYSCALL)
> + goto bad;
> + callp += code;
>
> i = callp->sy_argsize / sizeof(register_t);
> if (i > sizeof(args) / sizeof(register_t))
> @@ -1200,7 +1190,7 @@ m88100_syscall(register_t code, struct t
> rval[0] = 0;
> rval[1] = tf->tf_r[3];
>
> - error = mi_syscall(p, code, indirect, callp, args, rval);
> + error = mi_syscall(p, code, callp, args, rval);
>
> /*
> * system call will look like:
> @@ -1266,7 +1256,7 @@ void
> m88110_syscall(register_t code, struct trapframe *tf)
> {
> int i, nap;
> - const struct sysent *callp;
> + const struct sysent *callp = sysent;
> struct proc *p = curproc;
> int error;
> register_t args[8] __aligned(8);
> @@ -1285,17 +1275,8 @@ m88110_syscall(register_t code, struct t
> ap = &tf->tf_r[2];
> nap = 8; /* r2-r9 */
>
> - switch (code) {
> - case SYS_syscall:
> - code = *ap++;
> - nap--;
> - break;
> - }
> -
> - callp = sysent;
> - if (code < 0 || code >= SYS_MAXSYSCALL)
> - callp += SYS_syscall;
> - else
> + // XXX out of range stays on syscall0, which we assume is enosys
> + if (code >= 0 || code <= SYS_MAXSYSCALL)
> callp += code;
>
> i = callp->sy_argsize / sizeof(register_t);
Shouldn't this be
if (code > 0 && code < SYS_MAXSYSCALL)
?
All the other places in the diff modify callp under that condition. Also
all the values of code >= SYS_MAXSYSCALL will be covered by code >= 0.
Out of curiosity, any reason why m88k doesn't do the goto bad early on?
Other than RTFM for the calling convention.
> Index: sys/arch/mips64/mips64/trap.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/mips64/mips64/trap.c,v
> diff -u -p -u -r1.167 trap.c
> --- sys/arch/mips64/mips64/trap.c 26 Apr 2023 16:53:59 -0000 1.167
> +++ sys/arch/mips64/mips64/trap.c 27 Oct 2023 03:17:44 -0000
> @@ -396,14 +396,12 @@ fault_common_no_miss:
> case T_SYSCALL+T_USER:
> {
> struct trapframe *locr0 = p->p_md.md_regs;
> - const struct sysent *callp;
> - unsigned int code, indirect = -1;
> + const struct sysent *callp = sysent;
> + unsigned int code;
> register_t tpc;
> uint32_t branch = 0;
> int error, numarg;
> - struct args {
> - register_t i[8];
> - } args;
> + register_t args[8];
> register_t rval[2];
>
> atomic_inc_int(&uvmexp.syscalls);
> @@ -422,51 +420,22 @@ fault_common_no_miss:
> trapframe->pc, 0, branch);
> } else
> locr0->pc += 4;
> - callp = sysent;
> code = locr0->v0;
> - switch (code) {
> - case SYS_syscall:
> - /*
> - * Code is first argument, followed by actual args.
> - */
> - indirect = code;
> - code = locr0->a0;
> - if (code >= SYS_MAXSYSCALL)
> - callp += SYS_syscall;
> - else
> - callp += code;
> - numarg = callp->sy_argsize / sizeof(register_t);
> - args.i[0] = locr0->a1;
> - args.i[1] = locr0->a2;
> - args.i[2] = locr0->a3;
> - if (numarg > 3) {
> - args.i[3] = locr0->a4;
> - args.i[4] = locr0->a5;
> - args.i[5] = locr0->a6;
> - args.i[6] = locr0->a7;
> - if (numarg > 7)
> - if ((error = copyin((void *)locr0->sp,
> - &args.i[7], sizeof(register_t))))
> - goto bad;
> - }
> - break;
> - default:
> - if (code >= SYS_MAXSYSCALL)
> - callp += SYS_syscall;
> - else
> - callp += code;
> -
> - numarg = callp->sy_narg;
> - args.i[0] = locr0->a0;
> - args.i[1] = locr0->a1;
> - args.i[2] = locr0->a2;
> - args.i[3] = locr0->a3;
> - if (numarg > 4) {
> - args.i[4] = locr0->a4;
> - args.i[5] = locr0->a5;
> - args.i[6] = locr0->a6;
> - args.i[7] = locr0->a7;
> - }
> +
> + if (code <= 0 || code >= SYS_MAXSYSCALL)
> + goto bad;
> + callp += code;
> +
> + numarg = callp->sy_narg;
> + args[0] = locr0->a0;
> + args[1] = locr0->a1;
> + args[2] = locr0->a2;
> + args[3] = locr0->a3;
> + if (numarg > 4) {
> + args[4] = locr0->a4;
> + args[5] = locr0->a5;
> + args[6] = locr0->a6;
> + args[7] = locr0->a7;
> }
>
> rval[0] = 0;
> @@ -477,29 +446,24 @@ fault_common_no_miss:
> TRAPSIZE : trppos[ci->ci_cpuid]) - 1].code = code;
> #endif
>
> - error = mi_syscall(p, code, indirect, callp, args.i, rval);
> + error = mi_syscall(p, code, callp, args, rval);
>
> switch (error) {
> case 0:
> locr0->v0 = rval[0];
> locr0->a3 = 0;
> break;
> -
> case ERESTART:
> locr0->pc = tpc;
> break;
> -
> case EJUSTRETURN:
> break; /* nothing to do */
> -
> default:
> - bad:
Previous chunk gotos bad.
> locr0->v0 = error;
> locr0->a3 = 1;
> }
>
> mi_syscall_return(p, code, error, rval);
> -
> return;
> }
>
