On Fri, Sep 06, 2024 at 10:46:00AM -0700, Andrii Nakryiko wrote:

On Fri, Sep 6, 2024 at 2:39 AM Mark Rutland [off-list ref] wrote:

quoted

On Tue, Aug 27, 2024 at 07:33:55PM +0800, Liao, Chang wrote:

quoted

Hi, Mark

Would you like to discuss this patch further, or do you still believe emulating
STP to push FP/LR into the stack in kernel is not a good idea?

I'm happy with the NOP emulation in principle, so please send a new
version with *just* the NOP emulation, and I can review that.

Let's definitely start with that, this is important for faster USDT tracing.

quoted

Regarding STP emulation, I stand by my earlier comments, and in addition
to those comments, AFAICT it's currently unsafe to use any uaccess
routine in the uprobe BRK handler anyway, so that's moot. The uprobe BRK
handler runs with preemption disabled and IRQs (and all other maskable
exceptions) masked, and faults cannot be handled. IIUC
CONFIG_DEBUG_ATOMIC_SLEEP should scream about that.

This part I don't really get, and this might be some very
ARM64-specific issue, so I'm sorry ahead of time.

But in general, at the lowest level uprobes work in two logical steps.
First, there is a breakpoint that user space hits, kernel gets
control, and if VMA which hit breakpoint might contain uprobe, kernel
sets TIF_UPROBE thread flag and exits. This is the only part that's in
hard IRQ context. See uprobe_notify_resume() and comments around it.

Then uprobe infrastructure gets called in user context on the way back
to user space. This is where we confirm that this is uprobe/uretprobe
hit, and, if supported, perform instruction emulation.

So I'm wondering if your above comment refers to instruction emulation
within the first part of uprobe handling? If yes, then, no, that's not
where emulation will happen.

You're right -- I had misunderstood that the emulation happened during
handling of the breakpoint, rather than on the return-to-userspace path.
Looking at the arm64 entry code, the way uprobe_notify_resume() is
plumbed in is safe as it happens after we've re-enabled preemption and
unmasked other exceptions.

Sorry about that.

For the moment I'd still prefer to get the NOP case out of the way
first, so I'll review the NOP-only patch shortly.

Mark.