uprobes: Fix UPROBE_SKIP_SSTEP checks in handle_swbp() (0578a970) · Commits · 戴 / test

If handle_swbp()->add_utask() fails but UPROBE_SKIP_SSTEP is set, cleanup_ret: path do not restart the insn, this is wrong. Remove this check and add the additional label for can_skip_sstep() = T case. Note also that UPROBE_SKIP_SSTEP can be false positive, we simply can not trust it unless arch_uprobe_skip_sstep() was already called. Also, move another UPROBE_SKIP_SSTEP check before can_skip_sstep() into this helper, this looks more clean and understandable. Note: probably we should rename "skip" to "emulate" and I think that "clear UPROBE_SKIP_SSTEP" should be moved to arch_can_skip. Signed-off-by:

Oleg Nesterov <oleg@redhat.com> Acked-by:

Srikar Dronamraju <srikar@linux.vnet.ibm.com>

kernel/events/uprobes.c

+15 −16

Original line number	Diff line number	Diff line
		@@ -1389,10 +1389,11 @@ bool uprobe_deny_signal(void)
		*/
		static bool can_skip_sstep(struct uprobe uprobe, struct pt_regs regs)
		{
		if (uprobe->flags & UPROBE_SKIP_SSTEP) {
		if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
		return true;

		uprobe->flags &= ~UPROBE_SKIP_SSTEP;
		}
		return false;
		}

		@@ -1494,12 +1495,12 @@ static void handle_swbp(struct pt_regs *regs)
		utask = add_utask();
		/* Cannot allocate; re-execute the instruction. */
		if (!utask)
		goto cleanup_ret;
		goto restart;
		}

		handler_chain(uprobe, regs);
		if (uprobe->flags & UPROBE_SKIP_SSTEP && can_skip_sstep(uprobe, regs))
		goto cleanup_ret;
		if (can_skip_sstep(uprobe, regs))
		goto out;

		if (!pre_ssout(uprobe, regs, bp_vaddr)) {
		arch_uprobe_enable_step(&uprobe->arch);
		@@ -1508,15 +1509,13 @@ static void handle_swbp(struct pt_regs *regs)
		return;
		}

		cleanup_ret:
		if (!(uprobe->flags & UPROBE_SKIP_SSTEP))

		restart:
		/*
		* cannot singlestep; cannot skip instruction;
		* re-execute the instruction.
		*/
		instruction_pointer_set(regs, bp_vaddr);

		out:
		put_uprobe(uprobe);
		}

Admin message