Message ID | 20200116115700.127951-1-mkysel@tachyum.com |
---|---|
State | New |
Headers | show |
Series | Handling SIGSETXID used by glibc NPTL setuid/setgid | expand |
Hi all, It seems my patch was ignore. Ping! https://patchew.org/QEMU/20200116115700.127951-1-mkysel@tachyum.com/ -----Original Message----- From: Matus Kysel <mkysel@tachyum.com> Sent: štvrtok 16. januára 2020 12:57 Cc: Matus Kysel <mkysel@tachyum.com>; Riku Voipio <riku.voipio@iki.fi>; Laurent Vivier <laurent@vivier.eu>; open list:All patches CC here <qemu-devel@nongnu.org> Subject: [PATCH] Handling SIGSETXID used by glibc NPTL setuid/setgid Used same style to handle another glibc reserved signal SIGSETXID (33), that is used by glibc NPTL setuid/setgid functions. This should fix problems with application using those functions and failing with error "qemu:handle_cpu_signal received signal outside vCPU context". Signed-off-by: Matus Kysel <mkysel@tachyum.com> --- linux-user/signal.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 0128bde4d2..c59221fd0a 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = { [SIGPWR] = TARGET_SIGPWR, [SIGSYS] = TARGET_SIGSYS, /* next signals stay the same */ - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with - host libpthread signals. This assumes no one actually uses SIGRTMAX :-/ - To fix this properly we need to do manual signal delivery multiplexed - over a single host signal. */ + /* + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and SIGRTMAX - 1 + * to avoid overlap with host libpthread (NPTL glibc) signals. + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ + * To fix this properly we need to do manual signal delivery multiplexed + * over a single host signal. + */ [__SIGRTMIN] = __SIGRTMAX, + [__SIGRTMIN + 1] = __SIGRTMAX - 1, + [__SIGRTMAX - 1] = __SIGRTMIN + 1, [__SIGRTMAX] = __SIGRTMIN, }; static uint8_t target_to_host_signal_table[_NSIG]; -- 2.17.1
On Thu, 16 Jan 2020 at 11:58, Matus Kysel <mkysel@tachyum.com> wrote: > > Used same style to handle another glibc reserved signal SIGSETXID (33), > that is used by glibc NPTL setuid/setgid functions. This should fix problems > with application using those functions and failing with error > "qemu:handle_cpu_signal received signal outside vCPU context". > > Signed-off-by: Matus Kysel <mkysel@tachyum.com> > --- > linux-user/signal.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 0128bde4d2..c59221fd0a 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = { > [SIGPWR] = TARGET_SIGPWR, > [SIGSYS] = TARGET_SIGSYS, > /* next signals stay the same */ > - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with > - host libpthread signals. This assumes no one actually uses SIGRTMAX :-/ > - To fix this properly we need to do manual signal delivery multiplexed > - over a single host signal. */ > + /* > + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and SIGRTMAX - 1 > + * to avoid overlap with host libpthread (NPTL glibc) signals. > + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ > + * To fix this properly we need to do manual signal delivery multiplexed > + * over a single host signal. > + */ > [__SIGRTMIN] = __SIGRTMAX, > + [__SIGRTMIN + 1] = __SIGRTMAX - 1, > + [__SIGRTMAX - 1] = __SIGRTMIN + 1, > [__SIGRTMAX] = __SIGRTMIN, > }; > static uint8_t target_to_host_signal_table[_NSIG]; > -- > 2.17.1 This is a long-standing known problem, but doing this is likely to break currently-working guest binaries (notably things written in Go). See for example the discussion on this thread: https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg03804.html thanks -- PMM
I proposed a similar patch guarded by #ifdef TARGET_HEXAGON. I understand we don't want to break things that are working, and we don't want to open the door for a bunch of #ifdef's here. However, more than one scenario needs this capability. Would a suitable compromise be to provide a hook in linux-user/signal.c for the target to put in the changes they need? diff --git a/linux-user/hexagon/target_signal.h b/linux-user/hexagon/target_signal.h index ae52ff5..f82f8c1 100644 --- a/linux-user/hexagon/target_signal.h +++ b/linux-user/hexagon/target_signal.h @@ -31,4 +31,15 @@ typedef struct target_sigaltstack { #include "../generic/signal.h" + /* + * Hexagon uses the same signal for pthread cancel as the host pthreads, + * so cannot be overridden. + * Therefore, we map Hexagon signal to a different host signal. + */ +#define SIGNAL_MODS \ + [__SIGRTMIN + 1] = __SIGRTMAX - 1, \ + [__SIGRTMAX - 1] = __SIGRTMIN + 1, + + + #endif /* TARGET_SIGNAL_H */ diff --git a/linux-user/signal.c b/linux-user/signal.c index 5ca6d62..ebca770 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -24,6 +24,7 @@ #include "qemu.h" #include "trace.h" #include "signal-common.h" +#include "target_signal.h" static struct target_sigaction sigact_table[TARGET_NSIG]; @@ -72,6 +73,9 @@ static uint8_t host_to_target_signal_table[_NSIG] = { over a single host signal. */ [__SIGRTMIN] = __SIGRTMAX, [__SIGRTMAX] = __SIGRTMIN, +#ifdef SIGNAL_MODS + SIGNAL_MODS +#endif }; static uint8_t target_to_host_signal_table[_NSIG]; Taylor > -----Original Message----- > From: Qemu-devel <qemu-devel- > bounces+tsimpson=quicinc.com@nongnu.org> On Behalf Of Peter Maydell > Sent: Tuesday, January 28, 2020 3:27 AM > To: Matus Kysel <mkysel@tachyum.com> > Cc: Riku Voipio <riku.voipio@iki.fi>; open list:All patches CC here <qemu- > devel@nongnu.org>; Laurent Vivier <laurent@vivier.eu> > Subject: Re: [PATCH] Handling SIGSETXID used by glibc NPTL setuid/setgid > > ------------------------------------------------------------------------- > CAUTION: This email originated from outside of the organization. > ------------------------------------------------------------------------- > > On Thu, 16 Jan 2020 at 11:58, Matus Kysel <mkysel@tachyum.com> wrote: > > > > Used same style to handle another glibc reserved signal SIGSETXID > > (33), that is used by glibc NPTL setuid/setgid functions. This should > > fix problems with application using those functions and failing with > > error "qemu:handle_cpu_signal received signal outside vCPU context". > > > > Signed-off-by: Matus Kysel <mkysel@tachyum.com> > > --- > > linux-user/signal.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/linux-user/signal.c b/linux-user/signal.c index > > 0128bde4d2..c59221fd0a 100644 > > --- a/linux-user/signal.c > > +++ b/linux-user/signal.c > > @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = > { > > [SIGPWR] = TARGET_SIGPWR, > > [SIGSYS] = TARGET_SIGSYS, > > /* next signals stay the same */ > > - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with > > - host libpthread signals. This assumes no one actually uses SIGRTMAX :- > / > > - To fix this properly we need to do manual signal delivery multiplexed > > - over a single host signal. */ > > + /* > > + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and > SIGRTMAX - 1 > > + * to avoid overlap with host libpthread (NPTL glibc) signals. > > + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ > > + * To fix this properly we need to do manual signal delivery multiplexed > > + * over a single host signal. > > + */ > > [__SIGRTMIN] = __SIGRTMAX, > > + [__SIGRTMIN + 1] = __SIGRTMAX - 1, > > + [__SIGRTMAX - 1] = __SIGRTMIN + 1, > > [__SIGRTMAX] = __SIGRTMIN, > > }; > > static uint8_t target_to_host_signal_table[_NSIG]; > > -- > > 2.17.1 > > This is a long-standing known problem, but doing this is likely to break > currently-working guest binaries (notably things written in Go). See for > example the discussion on this thread: > https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg03804.html > > thanks > -- PMM
On Tue, 28 Jan 2020 at 12:33, Taylor Simpson <tsimpson@quicinc.com> wrote: > > I proposed a similar patch guarded by #ifdef TARGET_HEXAGON. I understand we don't want to break things that are working, and we don't want to open the door for a bunch of #ifdef's here. However, more than one scenario needs this capability. > > Would a suitable compromise be to provide a hook in linux-user/signal.c for the target to put in the changes they need? No; this isn't target-specific, and we don't want per-target different behaviour. thanks -- PMM
Le 28/01/2020 à 13:45, Peter Maydell a écrit : > On Tue, 28 Jan 2020 at 12:33, Taylor Simpson <tsimpson@quicinc.com> wrote: >> >> I proposed a similar patch guarded by #ifdef TARGET_HEXAGON. I understand we don't want to break things that are working, and we don't want to open the door for a bunch of #ifdef's here. However, more than one scenario needs this capability. >> >> Would a suitable compromise be to provide a hook in linux-user/signal.c for the target to put in the changes they need? > > No; this isn't target-specific, and we don't want > per-target different behaviour. So I guess the only way to fix the problem is to implement the signal multiplexing initiated by Milos [1] and tried again by Josh [2]. Any volunteer? If not, I will try to find some time to do that... Thanks, Laurent [1] "[PATCH 0/5] linux-user: Support signal passing for targets having more signals than host" https://patchew.org/QEMU/1558550785-20098-1-git-send-email-aleksandar.markovic@rt-rk.com/ [2] https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg00738.html
Le 28/01/2020 à 10:26, Peter Maydell a écrit : > On Thu, 16 Jan 2020 at 11:58, Matus Kysel <mkysel@tachyum.com> wrote: >> >> Used same style to handle another glibc reserved signal SIGSETXID (33), >> that is used by glibc NPTL setuid/setgid functions. This should fix problems >> with application using those functions and failing with error >> "qemu:handle_cpu_signal received signal outside vCPU context". >> >> Signed-off-by: Matus Kysel <mkysel@tachyum.com> >> --- >> linux-user/signal.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/linux-user/signal.c b/linux-user/signal.c >> index 0128bde4d2..c59221fd0a 100644 >> --- a/linux-user/signal.c >> +++ b/linux-user/signal.c >> @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = { >> [SIGPWR] = TARGET_SIGPWR, >> [SIGSYS] = TARGET_SIGSYS, >> /* next signals stay the same */ >> - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with >> - host libpthread signals. This assumes no one actually uses SIGRTMAX :-/ >> - To fix this properly we need to do manual signal delivery multiplexed >> - over a single host signal. */ >> + /* >> + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and SIGRTMAX - 1 >> + * to avoid overlap with host libpthread (NPTL glibc) signals. >> + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ >> + * To fix this properly we need to do manual signal delivery multiplexed >> + * over a single host signal. >> + */ >> [__SIGRTMIN] = __SIGRTMAX, >> + [__SIGRTMIN + 1] = __SIGRTMAX - 1, >> + [__SIGRTMAX - 1] = __SIGRTMIN + 1, >> [__SIGRTMAX] = __SIGRTMIN, >> }; >> static uint8_t target_to_host_signal_table[_NSIG]; >> -- >> 2.17.1 > > This is a long-standing known problem, but doing this is likely > to break currently-working guest binaries (notably things written > in Go). See for example the discussion on this thread: > https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg03804.html Peter, I try to fix this problem and I'd like to find a reproducer for the Go problem. I tried to write an "hello world" program and run it in an arm64/bionic chroot but there is no problem (with and without this patch). Any hints? Thanks, Laurent
Le 29/01/2020 à 17:12, Laurent Vivier a écrit : > Le 28/01/2020 à 10:26, Peter Maydell a écrit : >> On Thu, 16 Jan 2020 at 11:58, Matus Kysel <mkysel@tachyum.com> wrote: >>> >>> Used same style to handle another glibc reserved signal SIGSETXID (33), >>> that is used by glibc NPTL setuid/setgid functions. This should fix problems >>> with application using those functions and failing with error >>> "qemu:handle_cpu_signal received signal outside vCPU context". >>> >>> Signed-off-by: Matus Kysel <mkysel@tachyum.com> >>> --- >>> linux-user/signal.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/linux-user/signal.c b/linux-user/signal.c >>> index 0128bde4d2..c59221fd0a 100644 >>> --- a/linux-user/signal.c >>> +++ b/linux-user/signal.c >>> @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = { >>> [SIGPWR] = TARGET_SIGPWR, >>> [SIGSYS] = TARGET_SIGSYS, >>> /* next signals stay the same */ >>> - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with >>> - host libpthread signals. This assumes no one actually uses SIGRTMAX :-/ >>> - To fix this properly we need to do manual signal delivery multiplexed >>> - over a single host signal. */ >>> + /* >>> + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and SIGRTMAX - 1 >>> + * to avoid overlap with host libpthread (NPTL glibc) signals. >>> + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ >>> + * To fix this properly we need to do manual signal delivery multiplexed >>> + * over a single host signal. >>> + */ >>> [__SIGRTMIN] = __SIGRTMAX, >>> + [__SIGRTMIN + 1] = __SIGRTMAX - 1, >>> + [__SIGRTMAX - 1] = __SIGRTMIN + 1, >>> [__SIGRTMAX] = __SIGRTMIN, >>> }; >>> static uint8_t target_to_host_signal_table[_NSIG]; >>> -- >>> 2.17.1 >> >> This is a long-standing known problem, but doing this is likely >> to break currently-working guest binaries (notably things written >> in Go). See for example the discussion on this thread: >> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg03804.html > > Peter, > > I try to fix this problem and I'd like to find a reproducer for the Go > problem. > > I tried to write an "hello world" program and run it in an arm64/bionic > chroot but there is no problem (with and without this patch). > > Any hints? Markus has sent me the information. I'm able to reproduce the problem with an "Hello World" go program and go 1.10 (basically ubuntu bionic/arm64). Thanks, Laurent
diff --git a/linux-user/signal.c b/linux-user/signal.c index 0128bde4d2..c59221fd0a 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -66,11 +66,16 @@ static uint8_t host_to_target_signal_table[_NSIG] = { [SIGPWR] = TARGET_SIGPWR, [SIGSYS] = TARGET_SIGSYS, /* next signals stay the same */ - /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with - host libpthread signals. This assumes no one actually uses SIGRTMAX :-/ - To fix this properly we need to do manual signal delivery multiplexed - over a single host signal. */ + /* + * Nasty hack: Swap SIGRTMIN and SIGRTMIN + 1 with SIGRTMAX and SIGRTMAX - 1 + * to avoid overlap with host libpthread (NPTL glibc) signals. + * This assumes no one actually uses SIGRTMAX and SIGRTMAX - 1 :-/ + * To fix this properly we need to do manual signal delivery multiplexed + * over a single host signal. + */ [__SIGRTMIN] = __SIGRTMAX, + [__SIGRTMIN + 1] = __SIGRTMAX - 1, + [__SIGRTMAX - 1] = __SIGRTMIN + 1, [__SIGRTMAX] = __SIGRTMIN, }; static uint8_t target_to_host_signal_table[_NSIG];
Used same style to handle another glibc reserved signal SIGSETXID (33), that is used by glibc NPTL setuid/setgid functions. This should fix problems with application using those functions and failing with error "qemu:handle_cpu_signal received signal outside vCPU context". Signed-off-by: Matus Kysel <mkysel@tachyum.com> --- linux-user/signal.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)