On Thu, Sep 04, 2025 at 11:39:33AM +0200, Jann Horn wrote:

On Thu, Sep 4, 2025 at 9:56 AM Jiri Olsa [off-list ref] wrote:

quoted

On Wed, Sep 03, 2025 at 04:12:37PM -0700, Andrii Nakryiko wrote:

quoted

On Wed, Sep 3, 2025 at 2:01 PM Peter Zijlstra [off-list ref] wrote:

quoted

On Wed, Sep 03, 2025 at 10:56:10PM +0200, Jiri Olsa wrote:

quoted

quoted

quoted

+SYSCALL_DEFINE0(uprobe)
+{
+       struct pt_regs *regs = task_pt_regs(current);
+       struct uprobe_syscall_args args;
+       unsigned long ip, sp;
+       int err;
+
+       /* Allow execution only from uprobe trampolines. */
+       if (!in_uprobe_trampoline(regs->ip))
+               goto sigill;

Hey Jiri,

So I've been thinking what's the simplest and most reliable way to
feature-detect support for this sys_uprobe (e.g., for libbpf to know
whether we should attach at nop5 vs nop1), and clearly that would be
to try to call uprobe() syscall not from trampoline, and expect some
error code.

How bad would it be to change this part to return some unique-enough
error code (-ENXIO, -EDOM, whatever).

Is there any reason not to do this? Security-wise it will be just fine, right?

good question.. maybe :) the sys_uprobe sigill error path followed the
uprobe logic when things go bad, seem like good idea to be strict

I understand it'd make the detection code simpler, but it could just
just fork and check for sigill, right?

Can't you simply uprobe your own nop5 and read back the text to see what
it turns into?

Sure, but none of that is neither fast, nor cheap, nor that simple...
(and requires elevated permissions just to detect)

Forking is also resource-intensive. (think from libbpf's perspective,
it's not cool for library to fork some application just to check such
a seemingly simple thing as whether to

The question is why all that? That SIGILL when !in_uprobe_trampoline()
is just paranoid. I understand killing an application if it tries to
screw up "protocol" in all the subsequent checks. But here it's
equally secure to just fail that syscall with normal error, instead of
punishing by death.

adding Jann to the loop, any thoughts on this ^^^ ?

If I understand correctly, the main reason for the SIGILL is that if
you hit an error in here when coming from an actual uprobe, and if the
syscall were to just return an error, then you'd end up not restoring
registers as expected which would probably end up crashing the process
in a pretty ugly way?

for some cases yes, for the initial checks I think we could just skip
the uprobe and process would continue just fine

we use sigill because the trap code paths use it for errors and to be
paranoid about the !in_uprobe_trampoline check

jirka

I guess as long as in_uprobe_trampoline() is reliable (which it should
be), it would be fine to return an error when in_uprobe_trampoline()
fails, though it would be nice to have a short comment describing that
calls from uprobe trampolines must never fail with an error.