On 17 October 2012 23:06, Alex Barcelo <abarc...@ac.upc.edu> wrote: > On Wed, Oct 17, 2012 at 5:01 PM, Peter Maydell <peter.mayd...@linaro.org> > wrote: >> In my comments on v1 of this patch I wrote: >> "I think all the uses of sigprocmask() in linux-user/signal.c also >> need to be do_sigprocmask(), as they are the guest trying to control >> its signal mask (via the mask it specifies for running signal handlers, >> or the mask it passes back when restoring context on return from a >> signal handler)." > > I saw sigprocmask being used only inside sigreturn functions (hope I > checked it correctly). I thought (maybe wrongly) that sigreturn should > not be wrapped, just as internal sigprocmask calls are not proxyfied > through do_sigprocmask. > > Sigreturn functions should not be called from guest directly, so they > should not be a threat. And if some application uses it... well, then > it is its fault, as POSIX does not guarantee any behavior (am I > right?)
sigreturn functions operate on a structure passed in from the guest. When the kernel enters a signal handler it stores the state of the interrupted process on that process' stack before setting it up to run the signal handler function. Return from the signal handler involves the kernel (or in this case QEMU) restoring all that state. Part of that state is the signal mask. Since the guest might have changed that state, we must not trust it and so it goes through the wrapper. > I can't tell if it is worth doing it, or if there is any real case > where qemu-user behavior is improving. The point is consistency of design. Masks from the guest go through the wrapper; masks used internally do not. PS: if you disagree with a point in code review (and reviewers are not always right!) it is better to send an email making the case for why you disagree. If you just ignore it and send v2 patches then you're forcing reviewers to hunt through your patch all over again to check whether you paid attention the first time round... -- PMM