On Mon, Aug 04, 2025 at 10:12:15AM +0200, Jiri Olsa wrote:
> On Sat, Aug 02, 2025 at 12:34:27PM +0200, Oleg Nesterov wrote:
> > On 08/01, Jiri Olsa wrote:
> > >
> > > If uprobe handler changes instruction pointer we still execute single
> > > step) or emulate the original instruction and increment the (new) ip
> > > with its length.
> >
> > Yes... but what if we there are multiple consumers? The 1st one changes
> > instruction_pointer, the next is unaware. Or it may change regs->ip too...
>
> right, and I think that's already bad in current code
>
> how about we dd flag to the consumer that ensures it's the only consumer
> on the uprobe.. and we would skip original instruction execution for such
> uprobe if its consumer changes the regs->ip.. I'll try to come up with the
> patch
how about something like below?
jirka
---
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 516217c39094..b2c49a2d5468 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -59,6 +59,7 @@ struct uprobe_consumer {
struct list_head cons_node;
__u64 id; /* set when uprobe_consumer is registered */
+ bool is_unique; /* the only consumer on uprobe */
};
#ifdef CONFIG_UPROBES
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f774367c8e71..b317f9fbbf5c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1014,14 +1014,32 @@ static struct uprobe *alloc_uprobe(struct inode *inode,
loff_t offset,
return uprobe;
}
-static void consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
+static bool consumer_can_add(struct list_head *head, struct uprobe_consumer
*uc)
+{
+ /* there's no consumer, free to add one */
+ if (list_empty(head))
+ return true;
+ /* uprobe has consumer(s), can't add unique one */
+ if (uc->is_unique)
+ return false;
+ /* uprobe has consumer(s), we can add one only if it's not unique
consumer */
+ return !list_first_entry(head, struct uprobe_consumer,
cons_node)->is_unique;
+}
+
+static int consumer_add(struct uprobe *uprobe, struct uprobe_consumer *uc)
{
static atomic64_t id;
+ int ret = -EBUSY;
down_write(&uprobe->consumer_rwsem);
+ if (!consumer_can_add(&uprobe->consumers, uc))
+ goto unlock;
list_add_rcu(&uc->cons_node, &uprobe->consumers);
uc->id = (__u64) atomic64_inc_return(&id);
+ ret = 0;
+unlock:
up_write(&uprobe->consumer_rwsem);
+ return ret;
}
/*
@@ -1410,7 +1428,12 @@ struct uprobe *uprobe_register(struct inode *inode,
return uprobe;
down_write(&uprobe->register_rwsem);
- consumer_add(uprobe, uc);
+ ret = consumer_add(uprobe, uc);
+ if (ret) {
+ put_uprobe(uprobe);
+ up_write(&uprobe->register_rwsem);
+ return ERR_PTR(ret);
+ }
ret = register_for_each_vma(uprobe, uc);
up_write(&uprobe->register_rwsem);
@@ -2522,7 +2545,7 @@ static bool ignore_ret_handler(int rc)
return rc == UPROBE_HANDLER_REMOVE || rc == UPROBE_HANDLER_IGNORE;
}
-static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
+static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs, bool
*is_unique)
{
struct uprobe_consumer *uc;
bool has_consumers = false, remove = true;
@@ -2536,6 +2559,8 @@ static void handler_chain(struct uprobe *uprobe, struct
pt_regs *regs)
__u64 cookie = 0;
int rc = 0;
+ *is_unique |= uc->is_unique;
+
if (uc->handler) {
rc = uc->handler(uc, regs, &cookie);
WARN(rc < 0 || rc > 2,
@@ -2685,6 +2710,7 @@ static void handle_swbp(struct pt_regs *regs)
{
struct uprobe *uprobe;
unsigned long bp_vaddr;
+ bool is_unique = false;
int is_swbp;
bp_vaddr = uprobe_get_swbp_addr(regs);
@@ -2739,7 +2765,10 @@ static void handle_swbp(struct pt_regs *regs)
if (arch_uprobe_ignore(&uprobe->arch, regs))
goto out;
- handler_chain(uprobe, regs);
+ handler_chain(uprobe, regs, &is_unique);
+
+ if (is_unique && instruction_pointer(regs) != bp_vaddr)
+ goto out;
if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
goto out;
