[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 62/74] linux-user: Split out rt_sigqueueinfo,
From: |
Aleksandar Markovic |
Subject: |
Re: [Qemu-devel] [PATCH v7 62/74] linux-user: Split out rt_sigqueueinfo, rt_tgsigqueueinfo |
Date: |
Mon, 20 May 2019 08:04:08 +0200 |
On May 19, 2019 11:25 PM, "Richard Henderson" <address@hidden>
wrote:
>
> This does drop the (questionable) siginfo_t printing.
> But since we already do not handle more important things
> in this area like sigset_t, this does not feel a loss.
>
What is the most questionable here is the reasoning “we don't handle one
thing, therefore let's drop another”.
If you find siginfo_t printing questionable, then provide a better one, not
remove it altogether.
I find that the current printing is useful in many circumstances, there is
a lot of energy of former contributors invested into it, and should not be
deleted just like that. It does feel like a loss.
Aleksandar
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> linux-user/syscall-defs.h | 2 +
> linux-user/strace.c | 138 -----------------------------------
> linux-user/syscall-sig.inc.c | 30 ++++++++
> linux-user/syscall.c | 26 -------
> linux-user/strace.list | 6 --
> 5 files changed, 32 insertions(+), 170 deletions(-)
>
> diff --git a/linux-user/syscall-defs.h b/linux-user/syscall-defs.h
> index 24289ed413..11851535e1 100644
> --- a/linux-user/syscall-defs.h
> +++ b/linux-user/syscall-defs.h
> @@ -196,8 +196,10 @@ SYSCALL_DEF(rt_sigaction, ARG_SIGNAL, ARG_PTR,
ARG_PTR, ARG_DEC);
> #endif
> SYSCALL_DEF(rt_sigpending, ARG_PTR, ARG_DEC);
> SYSCALL_DEF(rt_sigprocmask, ARG_SIGPROCMASKHOW, ARG_PTR, ARG_PTR,
ARG_DEC);
> +SYSCALL_DEF(rt_sigqueueinfo, ARG_DEC, ARG_SIGNAL, ARG_PTR);
> SYSCALL_DEF(rt_sigsuspend, ARG_PTR, ARG_DEC);
> SYSCALL_DEF(rt_sigtimedwait, ARG_PTR, ARG_PTR, ARG_PTR, ARG_DEC);
> +SYSCALL_DEF(rt_tgsigqueueinfo, ARG_DEC, ARG_DEC, ARG_SIGNAL, ARG_PTR);
> #if !defined(SYSCALL_TABLE) || defined(TARGET_NR_semctl)
> SYSCALL_DEF(semctl, ARG_DEC, ARG_DEC, ARG_DEC, ARG_HEX);
> #endif
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 886663af2e..2e70a3910c 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -167,93 +167,6 @@ static void print_si_code(int arg)
> gemu_log("%s", codename);
> }
>
> -static void get_target_siginfo(target_siginfo_t *tinfo,
> - const target_siginfo_t *info)
> -{
> - abi_ulong sival_ptr;
> -
> - int sig;
> - int si_errno;
> - int si_code;
> - int si_type;
> -
> - __get_user(sig, &info->si_signo);
> - __get_user(si_errno, &tinfo->si_errno);
> - __get_user(si_code, &info->si_code);
> -
> - tinfo->si_signo = sig;
> - tinfo->si_errno = si_errno;
> - tinfo->si_code = si_code;
> -
> - /* Ensure we don't leak random junk to the guest later */
> - memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));
> -
> - /* This is awkward, because we have to use a combination of
> - * the si_code and si_signo to figure out which of the union's
> - * members are valid. (Within the host kernel it is always possible
> - * to tell, but the kernel carefully avoids giving userspace the
> - * high 16 bits of si_code, so we don't have the information to
> - * do this the easy way...) We therefore make our best guess,
> - * bearing in mind that a guest can spoof most of the si_codes
> - * via rt_sigqueueinfo() if it likes.
> - *
> - * Once we have made our guess, we record it in the top 16 bits of
> - * the si_code, so that print_siginfo() later can use it.
> - * print_siginfo() will strip these top bits out before printing
> - * the si_code.
> - */
> -
> - switch (si_code) {
> - case SI_USER:
> - case SI_TKILL:
> - case SI_KERNEL:
> - /* Sent via kill(), tkill() or tgkill(), or direct from the
kernel.
> - * These are the only unspoofable si_code values.
> - */
> - __get_user(tinfo->_sifields._kill._pid,
&info->_sifields._kill._pid);
> - __get_user(tinfo->_sifields._kill._uid,
&info->_sifields._kill._uid);
> - si_type = QEMU_SI_KILL;
> - break;
> - default:
> - /* Everything else is spoofable. Make best guess based on signal
*/
> - switch (sig) {
> - case TARGET_SIGCHLD:
> - __get_user(tinfo->_sifields._sigchld._pid,
> - &info->_sifields._sigchld._pid);
> - __get_user(tinfo->_sifields._sigchld._uid,
> - &info->_sifields._sigchld._uid);
> - __get_user(tinfo->_sifields._sigchld._status,
> - &info->_sifields._sigchld._status);
> - __get_user(tinfo->_sifields._sigchld._utime,
> - &info->_sifields._sigchld._utime);
> - __get_user(tinfo->_sifields._sigchld._stime,
> - &info->_sifields._sigchld._stime);
> - si_type = QEMU_SI_CHLD;
> - break;
> - case TARGET_SIGIO:
> - __get_user(tinfo->_sifields._sigpoll._band,
> - &info->_sifields._sigpoll._band);
> - __get_user(tinfo->_sifields._sigpoll._fd,
> - &info->_sifields._sigpoll._fd);
> - si_type = QEMU_SI_POLL;
> - break;
> - default:
> - /* Assume a sigqueue()/mq_notify()/rt_sigqueueinfo() source.
*/
> - __get_user(tinfo->_sifields._rt._pid,
&info->_sifields._rt._pid);
> - __get_user(tinfo->_sifields._rt._uid,
&info->_sifields._rt._uid);
> - /* XXX: potential problem if 64 bit */
> - __get_user(sival_ptr,
&info->_sifields._rt._sigval.sival_ptr);
> - tinfo->_sifields._rt._sigval.sival_ptr = sival_ptr;
> -
> - si_type = QEMU_SI_RT;
> - break;
> - }
> - break;
> - }
> -
> - tinfo->si_code = deposit32(si_code, 16, 16, si_type);
> -}
> -
> static void print_siginfo(const target_siginfo_t *tinfo)
> {
> /* Print a target_siginfo_t in the format desired for printing
> @@ -1585,57 +1498,6 @@ print_fstat(const struct syscallname *name,
> #define print_fstat64 print_fstat
> #endif
>
> -#ifdef TARGET_NR_rt_sigqueueinfo
> -static void
> -print_rt_sigqueueinfo(const struct syscallname *name,
> - abi_long arg0, abi_long arg1, abi_long arg2,
> - abi_long arg3, abi_long arg4, abi_long arg5)
> -{
> - void *p;
> - target_siginfo_t uinfo;
> -
> - print_syscall_prologue(name);
> - print_raw_param("%d", arg0, 0);
> - print_signal(arg1, 0);
> - p = lock_user(VERIFY_READ, arg2, sizeof(target_siginfo_t), 1);
> - if (p) {
> - get_target_siginfo(&uinfo, p);
> - print_siginfo(&uinfo);
> -
> - unlock_user(p, arg2, 0);
> - } else {
> - print_pointer(arg2, 1);
> - }
> - print_syscall_epilogue(name);
> -}
> -#endif
> -
> -#ifdef TARGET_NR_rt_tgsigqueueinfo
> -static void
> -print_rt_tgsigqueueinfo(const struct syscallname *name,
> - abi_long arg0, abi_long arg1, abi_long arg2,
> - abi_long arg3, abi_long arg4, abi_long arg5)
> -{
> - void *p;
> - target_siginfo_t uinfo;
> -
> - print_syscall_prologue(name);
> - print_raw_param("%d", arg0, 0);
> - print_raw_param("%d", arg1, 0);
> - print_signal(arg2, 0);
> - p = lock_user(VERIFY_READ, arg3, sizeof(target_siginfo_t), 1);
> - if (p) {
> - get_target_siginfo(&uinfo, p);
> - print_siginfo(&uinfo);
> -
> - unlock_user(p, arg3, 0);
> - } else {
> - print_pointer(arg3, 1);
> - }
> - print_syscall_epilogue(name);
> -}
> -#endif
> -
> #ifdef TARGET_NR_syslog
> static void
> print_syslog_action(abi_ulong arg, int last)
> diff --git a/linux-user/syscall-sig.inc.c b/linux-user/syscall-sig.inc.c
> index 5f2c0ba499..774346838b 100644
> --- a/linux-user/syscall-sig.inc.c
> +++ b/linux-user/syscall-sig.inc.c
> @@ -191,6 +191,21 @@ SYSCALL_IMPL(rt_sigprocmask)
> return ret;
> }
>
> +SYSCALL_IMPL(rt_sigqueueinfo)
> +{
> + siginfo_t uinfo;
> + void *p;
> +
> + p = lock_user(VERIFY_READ, arg3, sizeof(target_siginfo_t), 1);
> + if (!p) {
> + return -TARGET_EFAULT;
> + }
> + target_to_host_siginfo(&uinfo, p);
> + unlock_user(p, arg3, 0);
> +
> + return get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo));
> +}
> +
> SYSCALL_IMPL(rt_sigsuspend)
> {
> CPUState *cpu = ENV_GET_CPU(cpu_env);
> @@ -252,6 +267,21 @@ SYSCALL_IMPL(rt_sigtimedwait)
> return ret;
> }
>
> +SYSCALL_IMPL(rt_tgsigqueueinfo)
> +{
> + siginfo_t uinfo;
> + void *p;
> +
> + p = lock_user(VERIFY_READ, arg4, sizeof(target_siginfo_t), 1);
> + if (!p) {
> + return -TARGET_EFAULT;
> + }
> + target_to_host_siginfo(&uinfo, p);
> + unlock_user(p, arg4, 0);
> +
> + return get_errno(sys_rt_tgsigqueueinfo(arg1, arg2, arg3, &uinfo));
> +}
> +
> #ifdef TARGET_NR_sigaction
> SYSCALL_IMPL(sigaction)
> {
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8a05d3e32a..e489d12103 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4240,32 +4240,6 @@ static abi_long do_syscall1(void *cpu_env, int
num, abi_long arg1,
> void *p;
>
> switch(num) {
> - case TARGET_NR_rt_sigqueueinfo:
> - {
> - siginfo_t uinfo;
> -
> - p = lock_user(VERIFY_READ, arg3, sizeof(target_siginfo_t),
1);
> - if (!p) {
> - return -TARGET_EFAULT;
> - }
> - target_to_host_siginfo(&uinfo, p);
> - unlock_user(p, arg3, 0);
> - ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo));
> - }
> - return ret;
> - case TARGET_NR_rt_tgsigqueueinfo:
> - {
> - siginfo_t uinfo;
> -
> - p = lock_user(VERIFY_READ, arg4, sizeof(target_siginfo_t),
1);
> - if (!p) {
> - return -TARGET_EFAULT;
> - }
> - target_to_host_siginfo(&uinfo, p);
> - unlock_user(p, arg4, 0);
> - ret = get_errno(sys_rt_tgsigqueueinfo(arg1, arg2, arg3,
&uinfo));
> - }
> - return ret;
> #ifdef TARGET_NR_sigreturn
> case TARGET_NR_sigreturn:
> if (block_signals()) {
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 0b2c057673..57445a8d81 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -926,15 +926,9 @@
> #ifdef TARGET_NR_rmdir
> { TARGET_NR_rmdir, "rmdir" , NULL, NULL, NULL },
> #endif
> -#ifdef TARGET_NR_rt_sigqueueinfo
> -{ TARGET_NR_rt_sigqueueinfo, "rt_sigqueueinfo" , NULL,
print_rt_sigqueueinfo, NULL },
> -#endif
> #ifdef TARGET_NR_rt_sigreturn
> { TARGET_NR_rt_sigreturn, "rt_sigreturn" , NULL, NULL, NULL },
> #endif
> -#ifdef TARGET_NR_rt_tgsigqueueinfo
> -{ TARGET_NR_rt_tgsigqueueinfo, "rt_tgsigqueueinfo" , NULL,
print_rt_tgsigqueueinfo, NULL },
> -#endif
> #ifdef TARGET_NR_sched_getaffinity
> { TARGET_NR_sched_getaffinity, "sched_getaffinity" , NULL, NULL, NULL },
> #endif
> --
> 2.17.1
>
>
- [Qemu-devel] [PATCH v7 60/74] linux-user: Split out sigsuspend, rt_sigsuspend, (continued)
- [Qemu-devel] [PATCH v7 60/74] linux-user: Split out sigsuspend, rt_sigsuspend, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 55/74] linux-user: Split out getsid, setsid, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 47/74] linux-user: Split out ioctl, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 54/74] linux-user: Split out getpgid, getpgrp, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 52/74] linux-user: Split out umask, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 59/74] linux-user: Split out sigpending, rt_sigpending, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 64/74] linux-user: Split out gethostname, sethostname, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 65/74] linux-user: Split out getrlimit, setrlimit, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 61/74] linux-user: Split out rt_sigtimedwait, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 62/74] linux-user: Split out rt_sigqueueinfo, rt_tgsigqueueinfo, Richard Henderson, 2019/05/19
- Re: [Qemu-devel] [PATCH v7 62/74] linux-user: Split out rt_sigqueueinfo, rt_tgsigqueueinfo,
Aleksandar Markovic <=
- [Qemu-devel] [PATCH v7 63/74] linux-user: Split out sigreturn, rt_sigreturn, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 66/74] linux-user: Split out getrusage, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 67/74] linux-user: Split out gettimeofday, settimeofday, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 69/74] linux-user: Split out pselect6, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 68/74] linux-user: Split out select, _newselect, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 70/74] linux-user: Split out symlink, symlinkat, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 71/74] linux-user: Split out swapon, swapoff, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 72/74] linux-user: Split out reboot, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 73/74] linux-user: Split out truncate, truncate64, ftruncate, ftruncate64, Richard Henderson, 2019/05/19
- [Qemu-devel] [PATCH v7 74/74] linux-user: Split out getpriority, setpriority, Richard Henderson, 2019/05/19