On 08/06, Liao, Chang wrote:
You're absolutely right. handle_signlestep() has chance to handle _DENY_SIGANL
unless it followed by setting TIF_UPROBE in uprobe_deny_signal(). This means
_DENY_SIGNAL is likey replaced during next uprobe single-stepping.
I believe introducing _DENY_SIGNAL as the immediate state between UTASK_SSTEP
and UTASK_SSTEP_ACK is still necessary. This allow uprobe_post_sstep_notifier()
to correctly restore TIF_SIGPENDING upon the completion of single-step.
A revised implementation would look like this:
Still looks "obviously wrong" to me... even the approach itself.
Perhaps I am wrong, yet another day when I can't even read emails on lkml
carefully, sorry.
But can you please send the patch which I could actually apply? This one
looks white-space damaged...
I'll try to reply with more details as soon I convince myself I fully
understand what does your patch actually do, but most probably not tomorrow.
Thanks,
Oleg.
quoted hunk ↗ jump to hunk
------------------%<------------------
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1980,6 +1980,7 @@ bool uprobe_deny_signal(void)
if (task_sigpending(t)) {
clear_tsk_thread_flag(t, TIF_SIGPENDING);
+ utask->state = UTASK_SSTEP_DENY_SIGNAL;
if (__fatal_signal_pending(t) || arch_uprobe_xol_was_trapped(t)) {
utask->state = UTASK_SSTEP_TRAPPED;@@ -2276,22 +2277,23 @@ static void handle_singlestep(struct uprobe_task *utask, struct pt_regs *regs)
int err = 0;
uprobe = utask->active_uprobe;
- if (utask->state == UTASK_SSTEP_ACK)
+ switch (utask->state) {
+ case UTASK_SSTEP_ACK:
err = arch_uprobe_post_xol(&uprobe->arch, regs);
- else if (utask->state == UTASK_SSTEP_TRAPPED)
+ break;
+ case UTASK_SSTEP_TRAPPED:
arch_uprobe_abort_xol(&uprobe->arch, regs);
- else
+ set_thread_flag(TIF_SIGPENDING);
+ break;
+ default:
WARN_ON_ONCE(1);
+ }
put_uprobe(uprobe);
utask->active_uprobe = NULL;
utask->state = UTASK_RUNNING;
xol_free_insn_slot(current);
- spin_lock_irq(¤t->sighand->siglock);
- recalc_sigpending(); /* see uprobe_deny_signal() */
- spin_unlock_irq(¤t->sighand->siglock);
-
if (unlikely(err)) {
uprobe_warn(current, "execute the probed insn, sending SIGILL.");
force_sig(SIGILL);@@ -2351,6 +2353,8 @@ int uprobe_post_sstep_notifier(struct pt_regs *regs)
/* task is currently not uprobed */
return 0;
+ if (utask->state == UTASK_SSTEP_DENY_SIGNAL)
+ set_thread_flag(TIF_SIGPENDING);
utask->state = UTASK_SSTEP_ACK;
set_thread_flag(TIF_UPROBE);
return 1;
------------------>%------------------
quoted
Oleg.
--
BR
Liao, Chang