Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <Yjmn/kVblV3TdoAq@elver.google.com> <87ee2uyr4z.fsf@email.froward.int.ebiederm.org>
In-Reply-To: <87ee2uyr4z.fsf@email.froward.int.ebiederm.org>
From:   Marco Elver <elver@google.com>
Date:   Tue, 22 Mar 2022 17:44:31 +0100
Message-ID: <CANpmjNN4UjUTB5x6-2T-+b7MY=oAYn37MKvQy-4jYh6JDeJuKg@mail.gmail.com>
Subject: Re: RFC: Use of user space handler vs. SIG_DFL on forced signals
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 22 Mar 2022 at 15:54, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Marco Elver <elver@google.com> writes:
>
> > Hello,
> >
> > Currently force_sig_info_to_task() will always unblock a blocked signal
> > but deliver the signal to SIG_DFL:
> >
> >       [...]
> >        * Note: If we unblock the signal, we always reset it to SIG_DFL,
> >        * since we do not want to have a signal handler that was blocked
> >        * be invoked when user space had explicitly blocked it.
> >       [...]
> >
> > Is this requirement part of the POSIX spec? Or is the intent simply to
> > attempt to do the least-bad thing?
>
> I have not found any POSIX language about this.
>
> The options are either we terminate the application, or the application
> spins forever re-triggering the trap.

Is this in case of things like SEGV? I think this doesn't quite apply
to us. The cause of the signal (perf event) is rather benign, and the
signal handler can deal with recursion.

[...]
> > For SIGTRAP on perf events we found this makes the situation worse,
> > since the cause of the signal wasn't an error condition, but explicitly
> > requested monitoring. In this case, we do in fact want delivery of the
> > signal to user space even if the signal is blocked, i.e.
> > force_sig_perf() should be an unblockable forced synchronous signal to
> > user space!
>
> Which is exactly what we have.  If you block it you get terminated.

Right, however, in this case we want to monitor/trace memory accesses
etc, and some 3rd party code such as a library being traced isn't
under our control.

What we can do instead is to intercept sigprocmask() and work around
the issue, but libc interception is brittle. :-/
We do just want to receive the signal, all the time.

[...]
> I think HANDLER_UNBLOCK is pretty much nonsense.
>
> A block signal very much means that userspace is not prepared to handle
> the signal.  So calling something that is not ready to be called can't
> work.  That is common sense, and I expect in POSIX as well.

The fundamental question is, if we have a valid signal handler, but
sigprocmask() is called, how do we still keep receiving signals for
SIGTRAP despite sigprocmask()?

Perhaps this is impossible without intercepting sigprocmask() in user
space, in which we'll need to find a different solution.

> I expect that either you are looking for something like what ptrace does
> with signal interruptions where another process is notified, and
> userspace does not need to be involved, or that this is a don't do that
> then.
>
> Or possibly you have some weird asynchronous signal thing happening and
> you are calling it synchronous.

Not quite. We need it to be synchronous, because we need to know the
precise instruction and potentially do some other stuff _before_
subsequent instructions.

A compromise might be to deliver synchronously normally, but when
blocked deliver asynchronously. But if the signal was delivered
asynchronously, we need to let the signal handler know delivery was
asynchronous, so that our tracing logic can recover and give up at
that point.

To do this indication if it was asynchronous, we probably need to
extend siginfo_t once more. Would that be reasonable?

Thanks,
-- Marco