On Wed, 22 May 2019 at 20:00, Aleksandar Markovic <[email protected]> wrote: > > From: Miloš Stojanović <[email protected]> > > Fix copying between the host and target signal sets for the case when > the target set is larger than the host set. > > sigismember() returns 1 if the specified signal number is a member of > the specified signal set, but it can also return -1 if an error occurs > (e.g. an out of range signal number is specified). All non-zero values > would cause the signal to be added, so a comparison with 1 is added to > assure that only the signals which are really in the set get added to > the other set. > > Signed-off-by: Miloš Stojanović <[email protected]> > Signed-off-by: Aleksandar Markovic <[email protected]> > --- > linux-user/signal.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 44b2d3b..c08a7fe 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -116,7 +116,7 @@ void host_to_target_sigset_internal(target_sigset_t *d, > int i; > target_sigemptyset(d); > for (i = 1; i <= TARGET_NSIG; i++) { > - if (sigismember(s, i)) { > + if (sigismember(s, i) == 1) { > target_sigaddset(d, host_to_target_signal(i)); > } > }
I think perhaps a better fix for this would be to correct the loop termination condition to be "i < _NSIG". What we're doing here is looking at all the host signals in s to see if we should be adding them to d, so what we should be looping over is the valid range of host signal numbers, not the valid range of target signal numbers. Then we don't need to care about what sigismember() does if passed an invalid signal number, because we'll never do that. I also think we may have some off-by-one errors in various comparisons with TARGET_NSIG and _NSIG: sometimes we compare with <=, ie taking _NSIG or TARGET_NSIG to be the highest valid signal number, and sometimes we compare with <, taking it to be one greater than the highest valid signal number. For instance compare the signal_init() code loop and the size of arrays like host_to_target_signal_table[] versus the loop condition in host_to_target_sigset_internal() and target_to_host_sigset_internal(). I *think* that the correct intepretation is "it's one greater than the highest valid number" and we should fix the conditions in: host_to_target_sigset_internal target_to_host_sigset_internal do_sigprocmask signal_init host_signal_handler do_sigaction process_pending_signals but somebody should definitely check whether I'm right or not. thanks -- PMM
