On 1/21/20 12:13 AM, Alex Bennée wrote: > > Richard Henderson <richard.hender...@linaro.org> writes: > >> On 1/20/20 1:48 AM, Alex Bennée wrote: >>>> + default: >>>> + sigsegv: >>> >>> this label looks a little extraneous. >>> >>> Otherwise: >>> >>> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> >>> >> >> Look a little further down: >> >>> + default: >>> + sigsegv: >>> + /* Like force_sig(SIGSEGV). */ >>> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0); >>> + return; >>> + } >>> + >>> + /* >>> + * Validate the return address. >>> + * Note that the kernel treats this the same as an invalid entry point. >>> + */ >>> + if (get_user_u64(caller, env->regs[R_ESP])) { >>> + goto sigsegv; >>> + } > > Wouldn't this read better: > > /* > * Validate the entry point. We have already validated the page > * during translation, now verify the offset. > */ > switch (env->eip & ~TARGET_PAGE_MASK) { > case 0x000: > syscall = TARGET_NR_gettimeofday; > break; > case 0x400: > syscall = TARGET_NR_time; > break; > case 0x800: > syscall = TARGET_NR_getcpu; > break; > default: > syscall = -1; > break; > } > > /* > * If we have an invalid entry point or an invalid return address we > * generate a SIGSEG. > */ > if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) { > gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0); > return; > } >
Only if you have a violent goto allergy. r~