[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 17/19] linux-user: Use both si_code and si_si
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH v2 17/19] linux-user: Use both si_code and si_signo when converting siginfo_t |
Date: |
Tue, 7 Jun 2016 21:22:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 |
Le 27/05/2016 à 16:51, Peter Maydell a écrit :
> The siginfo_t struct includes a union. The correct way to identify
> which fields of the union are relevant is complicated, 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 tswap_siginfo()
> later can use it. tswap_siginfo() then strips these top bits out
> before writing si_code to the guest (sign-extending the lower bits).
>
> This fixes a bug where fields were sometimes wrong; in particular
> the LTP kill10 test went into an infinite loop because its signal
> handler got a si_pid value of 0 rather than the pid of the sending
> process.
>
> As part of this change, we switch to using __put_user() in the
> tswap_siginfo code which writes out the byteswapped values to
> the target memory, in case the target memory pointer is not
> sufficiently aligned for the host CPU's requirements.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> linux-user/signal.c | 165
> ++++++++++++++++++++++++++++++++--------------
> linux-user/syscall_defs.h | 15 +++++
> 2 files changed, 131 insertions(+), 49 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index b21d6bf..8ea0cbf 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -17,6 +17,7 @@
> * along with this program; if not, see <http://www.gnu.org/licenses/>.
> */
> #include "qemu/osdep.h"
> +#include "qemu/bitops.h"
> #include <sys/ucontext.h>
> #include <sys/resource.h>
>
> @@ -274,70 +275,129 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> const siginfo_t *info)
> {
> int sig = host_to_target_signal(info->si_signo);
> + int si_code = info->si_code;
> + int si_type;
> tinfo->si_signo = sig;
> tinfo->si_errno = 0;
> tinfo->si_code = info->si_code;
>
> - if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
> - || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
> - /* Should never come here, but who knows. The information for
> - the target is irrelevant. */
> - tinfo->_sifields._sigfault._addr = 0;
> - } else if (sig == TARGET_SIGIO) {
> - tinfo->_sifields._sigpoll._band = info->si_band;
> - tinfo->_sifields._sigpoll._fd = info->si_fd;
> - } else if (sig == TARGET_SIGCHLD) {
> - tinfo->_sifields._sigchld._pid = info->si_pid;
> - tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> + /* 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 tswap_siginfo() later can use it.
> + * tswap_siginfo() will strip these top bits out before writing
> + * si_code to the guest (sign-extending the lower bits).
> + */
> +
> + 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.
> + */
> + tinfo->_sifields._kill._pid = info->si_pid;
> + tinfo->_sifields._kill._uid = info->si_uid;
> + si_type = QEMU_SI_KILL;
> + break;
> + default:
> + /* Everything else is spoofable. Make best guess based on signal */
> + switch (sig) {
> + case TARGET_SIGCHLD:
> + tinfo->_sifields._sigchld._pid = info->si_pid;
> + tinfo->_sifields._sigchld._uid = info->si_uid;
> + tinfo->_sifields._sigchld._status
> = host_to_target_waitstatus(info->si_status);
> - tinfo->_sifields._sigchld._utime = info->si_utime;
> - tinfo->_sifields._sigchld._stime = info->si_stime;
> - } else if (sig >= TARGET_SIGRTMIN) {
> - tinfo->_sifields._rt._pid = info->si_pid;
> - tinfo->_sifields._rt._uid = info->si_uid;
> - /* XXX: potential problem if 64 bit */
> - tinfo->_sifields._rt._sigval.sival_ptr
> + tinfo->_sifields._sigchld._utime = info->si_utime;
> + tinfo->_sifields._sigchld._stime = info->si_stime;
> + si_type = QEMU_SI_CHLD;
> + break;
> + case TARGET_SIGIO:
> + tinfo->_sifields._sigpoll._band = info->si_band;
> + tinfo->_sifields._sigpoll._fd = info->si_fd;
> + si_type = QEMU_SI_POLL;
> + break;
> + default:
> + /* Assume a sigqueue()/mq_notify()/rt_sigqueueinfo() source. */
> + tinfo->_sifields._rt._pid = info->si_pid;
> + tinfo->_sifields._rt._uid = info->si_uid;
> + /* XXX: potential problem if 64 bit */
> + tinfo->_sifields._rt._sigval.sival_ptr
> = (abi_ulong)(unsigned long)info->si_value.sival_ptr;
> + si_type = QEMU_SI_RT;
> + break;
> + }
> + break;
> }
> +
> + tinfo->si_code = deposit32(si_code, 16, 16, si_type);
> }
>
> static void tswap_siginfo(target_siginfo_t *tinfo,
> const target_siginfo_t *info)
> {
> - int sig = info->si_signo;
> - tinfo->si_signo = tswap32(sig);
> - tinfo->si_errno = tswap32(info->si_errno);
> - tinfo->si_code = tswap32(info->si_code);
> -
> - if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
> - || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
> - tinfo->_sifields._sigfault._addr
> - = tswapal(info->_sifields._sigfault._addr);
> - } else if (sig == TARGET_SIGIO) {
> - tinfo->_sifields._sigpoll._band
> - = tswap32(info->_sifields._sigpoll._band);
> - tinfo->_sifields._sigpoll._fd =
> tswap32(info->_sifields._sigpoll._fd);
> - } else if (sig == TARGET_SIGCHLD) {
> - tinfo->_sifields._sigchld._pid
> - = tswap32(info->_sifields._sigchld._pid);
> - tinfo->_sifields._sigchld._uid
> - = tswap32(info->_sifields._sigchld._uid);
> - tinfo->_sifields._sigchld._status
> - = tswap32(info->_sifields._sigchld._status);
> - tinfo->_sifields._sigchld._utime
> - = tswapal(info->_sifields._sigchld._utime);
> - tinfo->_sifields._sigchld._stime
> - = tswapal(info->_sifields._sigchld._stime);
> - } else if (sig >= TARGET_SIGRTMIN) {
> - tinfo->_sifields._rt._pid = tswap32(info->_sifields._rt._pid);
> - tinfo->_sifields._rt._uid = tswap32(info->_sifields._rt._uid);
> - tinfo->_sifields._rt._sigval.sival_ptr
> - = tswapal(info->_sifields._rt._sigval.sival_ptr);
> + int si_type = extract32(info->si_code, 16, 16);
> + int si_code = sextract32(info->si_code, 0, 16);
> +
> + __put_user(info->si_signo, &tinfo->si_signo);
> + __put_user(info->si_errno, &tinfo->si_errno);
> + __put_user(si_code, &tinfo->si_code);
> +
> + /* We can use our internal marker of which fields in the structure
> + * are valid, rather than duplicating the guesswork of
> + * host_to_target_siginfo_noswap() here.
> + */
> + switch (si_type) {
> + case QEMU_SI_KILL:
> + __put_user(info->_sifields._kill._pid, &tinfo->_sifields._kill._pid);
> + __put_user(info->_sifields._kill._uid, &tinfo->_sifields._kill._uid);
> + break;
> + case QEMU_SI_TIMER:
Where is coming from QEMU_SI_TIMER?
It is not used elsewhere.
Laurent
- Re: [Qemu-devel] [PATCH v2 17/19] linux-user: Use both si_code and si_signo when converting siginfo_t,
Laurent Vivier <=