On 21 June 2016 at 20:09, Laurent Vivier <laur...@vivier.eu> wrote: > > > Le 20/06/2016 à 16:50, Peter Maydell a écrit : >> Many syscalls which take a sigset_t argument also take an argument >> giving the size of the sigset_t. The kernel insists that this >> matches its idea of the type size and fails EINVAL if it is not. >> Implement this logic in QEMU. (This mostly just means some LTP test >> cases which check error cases now pass.) >> >> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> >> --- >> linux-user/syscall.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 45 insertions(+), 1 deletion(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 95eafeb..7b3d129 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -7592,7 +7592,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> #if defined(TARGET_ALPHA) >> struct target_sigaction act, oact, *pact = 0; >> struct target_rt_sigaction *rt_act; >> - /* ??? arg4 == sizeof(sigset_t). */ >> + >> + if (arg4 != sizeof(target_sigset_t)) { >> + ret = -TARGET_EINVAL; >> + break; >> + } >> if (arg2) { >> if (!lock_user_struct(VERIFY_READ, rt_act, arg2, 1)) >> goto efault; >> @@ -7616,6 +7620,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> struct target_sigaction *act; >> struct target_sigaction *oact; >> >> + if (arg4 != sizeof(target_sigset_t)) { >> + ret = -TARGET_EINVAL; >> + break; >> + } >> if (arg2) { >> if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) >> goto efault; >> @@ -7747,6 +7755,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> int how = arg1; >> sigset_t set, oldset, *set_ptr; >> >> + if (arg4 != sizeof(target_sigset_t)) { >> + ret = -TARGET_EINVAL; >> + goto fail; >> + } > > why "goto fail" instead of "break"? > [you're not in a switch()] > >> + >> if (arg2) { >> switch(how) { >> case TARGET_SIG_BLOCK: >> @@ -7797,6 +7810,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> case TARGET_NR_rt_sigpending: >> { >> sigset_t set; >> + >> + /* Yes, this check is >, not != like most. We follow the >> kernel's >> + * logic and it does it like this because it implements >> + * NR_sigpending through the same code path, and in that case >> + * the old_sigset_t is smaller in size. >> + */ >> + if (arg2 > sizeof(target_sigset_t)) { >> + return -TARGET_EINVAL; >> + } > > why "return" instead of "break"?
Yeah, there's no particular reason for these inconsistencies, so I guess we should just use 'break' everywhere. thanks -- PMM