Hi Nam,
On 12/20, Nam Cao wrote:
>
> > Can't the trivial patch below fix the problem?
>
> It can. In fact this is the original fix we had. I thought that checking a
> single "core_state" is simpler than checking 3 flags, oh well..
>
> Can you send a proper patch, or should I do it?
Can you send V
On 12/17, Oleg Nesterov wrote:
>
> On 11/06, Nam Cao wrote:
> >
> > @@ -534,6 +517,23 @@ static int do_task_stat(struct seq_file *m, struct
> > pid_namespace *ns,
> > ppid = task_tgid_nr_ns(task->real_parent, ns);
> >
On 11/06, Nam Cao wrote:
>
> @@ -534,6 +517,23 @@ static int do_task_stat(struct seq_file *m, struct
> pid_namespace *ns,
> ppid = task_tgid_nr_ns(task->real_parent, ns);
> pgid = task_pgrp_nr_ns(task, ns);
>
> + /*
> + * esp and eip are intenti
On 09/20, Anjali Kulkarni wrote:
>
> > On Sep 20, 2024, at 4:00 AM, Oleg Nesterov wrote:
> >
> > I don't think you can use task_struct->exit_code. If this task is ptraced,
> > it can be changed/cleared in, say, ptrace_stop() after PROC_CN_MCAST_NOTIFY.
>
On 09/19, Anjali Kulkarni wrote:
>
> @@ -413,6 +416,10 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
> if (msg->len == sizeof(*pinput)) {
> pinput = (struct proc_input *)msg->data;
> mc_op = pinput->mcast_op;
> + if (mc_op == PROC_CN_MCAST_NOTIFY
other email (too late for me today), but I agree
that "avoid register_rwsem in handler_chain" is obviously a good goal,
lets discuss the possible cleanups or even fixlets later, when this
series is already applied.
> On Fri, Aug 30, 2024 at 7:33 AM Oleg Nesterov
On 08/30, Jiri Olsa wrote:
>
> with this change the probe will not get removed in the attached test,
> it'll get 2 hits, without this change just 1 hit
I don't understand the code in tools/...bpf../ at all, can't comment,
> but I'm not sure it's a big problem, because seems like that's not the
>
On 08/29, Andrii Nakryiko wrote:
>
> On Thu, Aug 29, 2024 at 4:10 PM Jiri Olsa wrote:
> >
> > > @@ -2101,17 +2110,24 @@ static void handler_chain(struct uprobe *uprobe,
> > > struct pt_regs *regs)
> > > need_prep = true;
> > >
> > > remove &= rc;
> > > +
On 08/29, Andrii Nakryiko wrote:
>
> v3->v4:
> - added back consumer_rwsem into consumer_del(), which was accidentally
> omitted earlier (Jiri);
still,
Reviewed-by: Oleg Nesterov
#syz dup: [syzbot] [perf?] KASAN: slab-use-after-free Read in
__uprobe_unregister syzbot
https://lore.kernel.org/all/382d39061f59f...@google.com/
Hopefully fixed by
[PATCH tip/perf/core] bpf: fix use-after-free in bpf_uprobe_multi_link_attach()
https://lore.kernel.org/all/20240813152
On 08/19, Andrii Nakryiko wrote:
>
> On Mon, Aug 19, 2024 at 6:41 AM Oleg Nesterov wrote:
> >
> > On 08/12, Andrii Nakryiko wrote:
> > >
> > > Avoid taking refcount on uprobe in prepare_uretprobe(), instead take
> > > uretprobe-specific SRC
On 08/20, Zhen Lei wrote:
>
> Depending on the argument 'add', uprobe_apply() may be registering or
> unregistering a probe.
...
> /*
> - * uprobe_apply - unregister an already registered probe.
> - * @inode: the file in which the probe has to be removed.
> + * uprobe_apply - register a probe or
this needs a bit more trivial changes.
What do you think?
Oleg.
---
>From d7cb674eb6f7bb891408b2b6a5fb872a6c2f0f6c Mon Sep 17 00:00:00 2001
From: Oleg Nesterov
Date: Mon, 19 Aug 2024 15:34:55 +0200
Subject: [RFC PATCH] uprobe: kill uprobe_task->active_uprobe
Untested, not for inclus
> applied and I'd appreciate early feedback on the remaining 5 ones.
I didn't read the "RFC" patches yet, will try to do on weekend.
As for 1-8, I failed to find any problem:
Reviewed-by: Oleg Nesterov
On 08/13, Masami Hiramatsu wrote:
>
> > @@ -62,7 +63,7 @@ struct trace_uprobe {
> > struct uprobe *uprobe;
>
> BTW, what is this change? I couldn't cleanly apply this to the v6.11-rc3.
> Which tree would you working on? (I missed something?)
tip/perf/core
See
https://git.ke
ail.com
Acked-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
kernel/trace/bpf_trace.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4e391daafa64..90cd30e9723e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/
example per_cpu_sum() or nr_processes(), per_cpu() should work just
fine...
Other than that
Reviewed-by: Oleg Nesterov
On 08/13, Liao, Chang wrote:
>
>
> Oleg, your explaination is more accurate. So I will reword the commit log and
> quote some of your note like this:
Oh, please don't. I just tried to explain the history of this
spin_lock(siglock).
> Since we already have the lockless user of
> clear_thread_f
On 08/12, Andrii Nakryiko wrote:
>
> adding bpf ML, given it's bpf's code base
Thanks,
> On Mon, Aug 12, 2024 at 3:00 AM Oleg Nesterov wrote:
> >
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -3491,8 +349
al)
case it should be fine.
Oleg.
> Acked-by: Oleg Nesterov
> Signed-off-by: Liao Chang
> ---
> kernel/events/uprobes.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 73cc47708679..76a51a1f51e2 100
On 08/09, Liao Chang wrote:
>
> --- a/include/linux/uprobes.h
> +++ b/include/linux/uprobes.h
> @@ -75,6 +75,7 @@ struct uprobe_task {
>
> struct uprobe *active_uprobe;
> unsigned long xol_vaddr;
> + booldeny_signal;
On 08/11, Andrii Nakryiko wrote:
>
> On Sun, Aug 11, 2024 at 5:35 AM Oleg Nesterov wrote:
> >
> > Something like below on top of perf/core. But I don't like the usage of
> > "i" in the +error_unregister path...
> >
>
> Wouldn't the below
On 08/10, syzbot wrote:
>
> syzbot found the following issue on:
>
> HEAD commit:6a0e38264012 Merge tag 'for-6.11-rc2-tag' of git://git.ker..
> git tree: upstream
#syz test: upstream 6a0e38264012
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cd098846e251..5d9c96
On 08/11, Oleg Nesterov wrote:
>
> Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.
>
> bpf_link_prime() is called after the
>
> for (i = 0; i < cnt; i++) {
> uprobe_register(...);
> ...
> }
>
Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.
bpf_link_prime() is called after the
for (i = 0; i < cnt; i++) {
uprobe_register(...);
...
}
loop. If bpf_link_prime() fails, bpf_uprobe_multi_link_attach() just do
kvfree(uprobes) without
On 08/08, Oleg Nesterov wrote:
>
> On 07/31, Andrii Nakryiko wrote:
> >
> > static DEFINE_RWLOCK(uprobes_treelock);/* serialize rbtree access */
> > +static seqcount_rwlock_t uprobes_seqcount =
> > SEQCNT_RWLOCK_ZERO(uprobes_seqcount, &uprobes_
Hi Liao,
To be honest I didn't even try to look at your patch, sorry.
Because I think it would be better to delay it in an case. Until Andrii
finishes/pushes his optimization changes which (in particular) include
find_active_uprobe_rcu/etc.
Then you can rebease and re-benchmark your patch on top
On 08/07, Andrii Nakryiko wrote:
>
> @@ -1127,18 +1105,30 @@ void uprobe_unregister(struct uprobe *uprobe, struct
> uprobe_consumer *uc)
> int err;
>
> down_write(&uprobe->register_rwsem);
> - if (WARN_ON(!consumer_del(uprobe, uc))) {
> - err = -ENOENT;
> - } else
On 08/07, Andrii Nakryiko wrote:
>
> So, any ideas how we can end up with "corrupted" root on lockless
> lookup with rb_find_rcu()?
I certainly can't help ;) I know ABSOLUTELY NOTHING about rb or any
other tree.
But,
> This seems to be the very first lockless
> RB-tree lookup use case in the tre
On 07/31, Andrii Nakryiko wrote:
>
> static DEFINE_RWLOCK(uprobes_treelock); /* serialize rbtree access */
> +static seqcount_rwlock_t uprobes_seqcount =
> SEQCNT_RWLOCK_ZERO(uprobes_seqcount, &uprobes_treelock);
Just noticed... Why seqcount_rwlock_t?
find_uprobe_rcu() doesn't use read_seq
On 08/08, Liao, Chang wrote:
>
>
> 在 2024/8/8 18:28, Oleg Nesterov 写道:
> > --- x/kernel/events/uprobes.c
> > +++ x/kernel/events/uprobes.c
> > @@ -2308,9 +2308,10 @@ static void handle_singlestep(struct upr
> > utask->state = UTASK_RUNNING
On 08/08, Liao, Chang wrote:
>
> - pre_ssout() resets the deny signal flag
>
> - uprobe_deny_signal() sets the deny signal flag when TIF_SIGPENDING is
> cleared.
>
> - handle_singlestep() check the deny signal flag and restore TIF_SIGPENDING
> if necessary.
>
> Does this approach look corre
On 08/07, Andrii Nakryiko wrote:
>
> struct uprobe {
> - struct rb_node rb_node;/* node in the rb tree */
> + union {
> + struct rb_node rb_node;/* node in the rb tree
> */
> + struct rcu_head rcu;/* mutually ex
On 08/07, Andrii Nakryiko wrote:
>
> Ok, so it seems like rb_find_rcu() and rb_find_add_rcu() are not
> enough or are buggy. I managed to more or less reliably start
> reproducing a crash, which was bisected to exactly this change. My
> wild guess is that we'd need an rb_erase_rcu() variant or some
On 08/07, Andrii Nakryiko wrote:
>
> Are you going to send a patch with
> your bool flag proposal?
No, I will wait for the next version from Liao.
If he agrees with my comments. If not, we will continue the discussion in that
thread.
Note also another patch from Liao which removes ->siglock fro
On 08/07, Andrii Nakryiko wrote:
>
> Yes, I was waiting for more of Peter's comments, but I guess I'll just
> send a v2 today.
OK,
> I'll probably include the SRCU+timeout logic for
> return_instances, and maybe lockless VMA parts as well.
Well, feel free to do what you think right, but perhaps
On 07/31, Andrii Nakryiko wrote:
>
> Andrii Nakryiko (6):
> uprobes: revamp uprobe refcounting and lifetime management
> uprobes: protected uprobe lifetime with SRCU
> uprobes: get rid of enum uprobe_filter_ctx in uprobe filter callbacks
> uprobes: travers uprobe's consumer list locklessly
I guess you know this, but just in case...
On 07/31, Andrii Nakryiko wrote:
>
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -478,7 +478,8 @@ static void testmod_unregister_uprobe(void)
> mutex_lock(&testmod_up
e != UTASK_SSTEP);
And no, we shouldn't change this check into UTASK_SSTEP ||
UTASK_SSTEP_DENY_SIGNAL.
Again, the fact that uprobe_deny_signal() cleared TIF_SIGPENDING must not be the
new state.
Oleg.
On 08/06, Oleg Nesterov wrote:
>
> On 08/06, Liao, Chang wrote:
>
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
On 08/05, Andrii Nakryiko wrote:
>
> On Mon, Aug 5, 2024 at 8:59 AM Oleg Nesterov wrote:
> >
> > > int uprobe_apply(struct uprobe *uprobe, struct uprobe_consumer *uc, bool
> > > add)
> > > {
> > > struct uprobe_consumer *con;
> > &g
On 08/05, Andrii Nakryiko wrote:
>
> On Mon, Aug 5, 2024 at 6:44 AM Oleg Nesterov wrote:
> >
> > On 07/31, Andrii Nakryiko wrote:
> > >
> > > @@ -732,11 +776,13 @@ static struct uprobe *alloc_uprobe(struct inode
> > > *inode, loff_t offset,
> &
On 07/31, Andrii Nakryiko wrote:
>
> @@ -1120,17 +1098,19 @@ void uprobe_unregister(struct uprobe *uprobe, struct
> uprobe_consumer *uc)
> int err;
>
> down_write(&uprobe->register_rwsem);
> - if (WARN_ON(!consumer_del(uprobe, uc))) {
> - err = -ENOENT;
OK, I agree, th
LGTM, just a few notes...
On 07/31, Andrii Nakryiko wrote:
>
> @@ -59,6 +61,7 @@ struct uprobe {
> struct list_headpending_list;
> struct uprobe_consumer *consumers;
> struct inode*inode; /* Also hold a ref to inode */
> + struct rcu_head
On 07/31, Andrii Nakryiko wrote:
>
> @@ -732,11 +776,13 @@ static struct uprobe *alloc_uprobe(struct inode *inode,
> loff_t offset,
> uprobe->ref_ctr_offset = ref_ctr_offset;
> init_rwsem(&uprobe->register_rwsem);
> init_rwsem(&uprobe->consumer_rwsem);
> + RB_CLEAR_NODE(&upro
On 08/02, Andrii Nakryiko wrote:
>
> On Fri, Aug 2, 2024 at 1:50 AM Oleg Nesterov wrote:
> >
> > On 08/01, Andrii Nakryiko wrote:
> > >
> > > > + /* TODO : cant unregister? schedule a worker thread */
> > > > +
On 08/02, Liao, Chang wrote:
>
>
> 在 2024/8/1 22:06, Oleg Nesterov 写道:
> > On 08/01, Liao Chang wrote:
> >>
> >> @@ -2276,22 +2277,25 @@ static void handle_singlestep(struct uprobe_task
> >> *utask, struct pt_regs *regs)
> >>int err = 0;
>
On 08/01, Andrii Nakryiko wrote:
>
> > + /* TODO : cant unregister? schedule a worker thread */
> > + WARN(err, "leaking uprobe due to failed unregistration");
> Ok, so now that I added this very loud warning if
> register_for_each_vma(uprobe, NULL) returns error, it tu
On 08/01, Liao Chang wrote:
>
> @@ -2276,22 +2277,25 @@ 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_S
Kill the extra get_uprobe() + put_uprobe() in uprobe_unregister() and
move the possibly final put_uprobe() from delete_uprobe() to its only
caller, uprobe_unregister().
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
Acked-by: Masami Hiramatsu (Google)
Reviewed-by: Jiri Olsa
Fold __uprobe_unregister() into its single caller, uprobe_unregister().
A separate patch to simplify the next change.
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
Acked-by: Masami Hiramatsu (Google)
Reviewed-by: Jiri Olsa
---
kernel/events/uprobes.c | 25
ccessful _register().
Yes this means the extra up_write() + down_write(), but this is the
slow and unlikely case anyway.
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
Acked-by: Masami Hiramatsu (Google)
Reviewed-by: Jiri Olsa
---
kernel/events/uprobes.c | 12 +++-
1 file ch
that
this uprobe can't be freed before up_write(&uprobe->register_rwsem).
Co-developed-by: Andrii Nakryiko
Signed-off-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
Reviewed-by: Jiri Olsa
---
include/linux/uprobes.h | 15 ++---
kernel/events/uprobes.c
s argument to uprobe_register(), update the callers, and kill
uprobe_register_refctr().
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
Reviewed-by: Jiri Olsa
---
include/linux/uprobes.h | 9 ++-
kernel/events/uprobes.c | 24 +-
From: Jiri Olsa
testmod_unregister_uprobe() forgets to path_put(&uprobe.path).
Signed-off-by: Jiri Olsa
Signed-off-by: Oleg Nesterov
---
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/bpf_tes
From: Andrii Nakryiko
Return -ENOMEM instead of NULL, which makes caller's error handling just
a touch simpler.
Signed-off-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
Reviewed-by: Masami Hiramatsu (Google)
Reviewed-by: Jiri Olsa
---
kernel/events/uprobes.c | 4 +---
1 file chang
get_user_pages_remote() and the comment above it make no sense.
There is no task_struct passed into get_user_pages_remote() anymore, and
nowadays mm_account_fault() increments the current->min/maj_flt counters
regardless of FAULT_FLAG_REMOTE.
Reported-by: Andrii Nakryiko
Signed-off-by: O
is_trap_at_addr() which returns true
- T returns to handle_swbp() and gets SIGTRAP.
Reported-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
Acked-by: Masami Hiramatsu (Google)
Reviewed-by: Jiri Olsa
---
kernel/events/uprobes.c | 10 --
1 file changed, 8
(Andrii, I'll try to look at your new series on Weekend).
Changes:
- added the acks I got from Andrii, Masami, and Jiri
- new 4/9 patch from Jiri, fixes the unrelated bug in bpf_testmod
- adapt tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
to the API chan
ckport just the fix
OK, I'll rebase and add the patch below to v4. OK?
Oleg.
---
>From f6bf42015048938d826880e3bf4a318bb64a05b4 Mon Sep 17 00:00:00 2001
From: Oleg Nesterov
Date: Thu, 1 Aug 2024 14:21:47 +0200
Subject: [PATCH] selftests/bpf: fix uprobe.path leak in bpf_testmod
From:
On 08/01, Jiri Olsa wrote:
>
> > @@ -474,10 +477,10 @@ static void testmod_unregister_uprobe(void)
> > {
> > mutex_lock(&testmod_uprobe_mutex);
> >
> > - if (uprobe.offset) {
> > - uprobe_unregister(d_real_inode(uprobe.path.dentry),
> > -
On 07/31, Andrii Nakryiko wrote:
>
> My expectation was that Oleg will
> just incorporate that by hand and will send the final v4 patch set.
Will do tomorrow, thanks.
Oleg.
On 07/31, Peter Zijlstra wrote:
>
> On Wed, Jul 31, 2024 at 10:01:47AM -0700, Andrii Nakryiko wrote:
> > > Do I stuff this on top of Oleg's patch or do you want me to fold it in
> > > one of them?
> >
> > Please fold so we have better (potential) bisectability of BPF
> > selftests, thanks!
>
> Fold
ck_irq(&t->sighand->siglock);
> clear_tsk_thread_flag(t, TIF_SIGPENDING);
> - spin_unlock_irq(&t->sighand->siglock);
Agreed, in this case ->siglock buys nothing, another signal can come
right after spin_unlock().
Acked-by: Oleg Nesterov
s argument to uprobe_register(), update the callers, and kill
uprobe_register_refctr().
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
---
include/linux/uprobes.h | 9 ++-
kernel/events/uprobes.c | 24 +--
kernel/trac
On 07/31, Masami Hiramatsu wrote:
>
> OK, but it seems
>
> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
>
> is still using uprobe_register_refctr().
>
> That should be updated too.
OOPS, thanks a lot :/
I'll send v3 with the additional change below in reply to 4/5 in a minute.
Masami, P
On 07/31, Masami Hiramatsu wrote:
>
> On Mon, 29 Jul 2024 15:45:35 +0200
> Oleg Nesterov wrote:
>
> > This way uprobe_unregister() and uprobe_apply() can use "struct uprobe *"
> > rather than inode + offset. This simplifies the code and allows to avo
Thanks for looking at this!
On 07/30, Andrii Nakryiko wrote:
>
> BTW, do you have anything against me changing refcounting so that
> uprobes_tree itself doesn't hold a refcount, and all the refcounting
> is done based on consumers holding implicit refcount and whatever
> temporary get/put uprobe i
Kill the extra get_uprobe() + put_uprobe() in uprobe_unregister() and
move the possibly final put_uprobe() from delete_uprobe() to its only
caller, uprobe_unregister().
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions
Fold __uprobe_unregister() into its single caller, uprobe_unregister().
A separate patch to simplify the next change.
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/kernel/events/uprobes.c
ccessful _register().
Yes this means the extra up_write() + down_write(), but this is the
slow and unlikely case anyway.
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/u
On top of
[PATCH v2 0/5] uprobes: misc cleanups/simplifications
https://lore.kernel.org/all/2024072913.ga12...@redhat.com/
I sent yesterday.
Oleg.
---
kernel/events/uprobes.c | 47 ---
1 file changed, 24 insertions(+), 23 deletion
that
this uprobe can't be freed before up_write(&uprobe->register_rwsem).
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
---
include/linux/uprobes.h | 15 +-
kernel/events/uprobes.c | 56 +++--
kernel/trace/bpf_trace.c| 25 +++
s argument to uprobe_register(), update the callers, and kill
uprobe_register_refctr().
Signed-off-by: Oleg Nesterov
Acked-by: Andrii Nakryiko
---
include/linux/uprobes.h | 9 ++---
kernel/events/uprobes.c | 24 ++--
kernel/trace/bpf_trace.c| 8
From: Andrii Nakryiko
Return -ENOMEM instead of NULL, which makes caller's error handling just
a touch simpler.
Signed-off-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/e
rii Nakryiko
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 73dd12b09a7b..eb71428691bb 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2035
is_trap_at_addr() which returns true
- T returns to handle_swbp() and gets SIGTRAP.
Reported-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
Peter, I don't think these changes can really complicate your ongoing work.
But again, if you are going to send the next version "soon" I can rebase
these cleanups on top of it.
Andrii, I dared to preserve your acks, all the changes are simple.
Changes since v1:
- update the comment in r
On 07/15, Masami Hiramatsu wrote:
>
> Hi Peter, Oleg,
>
> If this is OK for you, please give your Ack.
Acked-by: Oleg Nesterov
On 07/11, Peter Zijlstra wrote:
>
> uprobe_free_stage1
> call_srcu(&uretprobe_srcu, &uprobe->rcu, uprobe_free_stage2);
>
> put_uprobe()
> if (refcount_dec_and_test)
> call_srcu(&uprobes_srcu, &uprobe->rcu, uprobe_free_stage1);
>
>
> So my thinking was since we take uretprobe_srcu
On 07/11, Andrii Nakryiko wrote:
>
> On Thu, Jul 11, 2024 at 2:28 AM Oleg Nesterov wrote:
> >
> > On 07/10, Oleg Nesterov wrote:
> > >
> > > -void uprobe_unregister(struct inode *inode, loff_t offset, struct
> > > uprobe_consumer *uc)
> > >
I'll try to actually apply the whole series and read the code tomorrow.
Right now I can't understand this change... Just one question for now.
On 07/11, Peter Zijlstra wrote:
>
> @@ -1956,11 +1960,13 @@ static void prepare_uretprobe(struct upr
>* attack from user-space.
>
Not sure I read this patch correctly, but at first glance it looks
suspicious..
On 07/11, Peter Zijlstra wrote:
>
> +static void return_instance_timer(struct timer_list *timer)
> +{
> + struct uprobe_task *utask = container_of(timer, struct uprobe_task,
> ri_timer);
> + task_work_add(utas
On 07/10, Oleg Nesterov wrote:
>
> -void uprobe_unregister(struct inode *inode, loff_t offset, struct
> uprobe_consumer *uc)
> +void uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *uc)
> {
> - struct uprobe *uprobe;
> -
> - uprobe = find_uprobe(in
On 07/11, Peter Zijlstra wrote:
>
> On Wed, Jul 10, 2024 at 06:30:22PM +0200, Oleg Nesterov wrote:
> >
> > In fact I would like to push 2 more cleanups before the more significant
> > changes, but they certainly conflict with your ongoing work, albeit only
> > text
On 07/10, Andrii Nakryiko wrote:
>
> On Wed, Jul 10, 2024 at 9:33 AM Oleg Nesterov wrote:
> >
> > This way uprobe_unregister() and uprobe_apply() do not need find_uprobe() +
> > put_uprobe(). And to me this change simplifies the code a bit.
> >
&g
On 07/10, Andrii Nakryiko wrote:
>
> LGTM with few nits below.
>
> Acked-by: Andrii Nakryiko
Thanks for looking at this.
> > @@ -3477,7 +3477,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr
> > *attr, struct bpf_prog *pr
> > &bpf_uprobe_multi_link_lops, prog);
>
On 07/10, Jiri Olsa wrote:
>
> > @@ -3180,10 +3181,8 @@ static void bpf_uprobe_unregister(struct path *path,
> > struct bpf_uprobe *uprobes,
> > {
> > u32 i;
> >
> > - for (i = 0; i < cnt; i++) {
> > - uprobe_unregister(d_real_inode(path->dentry), uprobes[i].offset,
> > -
This way uprobe_unregister() and uprobe_apply() do not need find_uprobe() +
put_uprobe(). And to me this change simplifies the code a bit.
Signed-off-by: Oleg Nesterov
---
include/linux/uprobes.h | 14 ++--
kernel/events/uprobes.c | 45
From: Andrii Nakryiko
Return -ENOMEM instead of NULL, which makes caller's error handling just
a touch simpler.
Signed-off-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
kernel/events/uprobes.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/e
s argument to uprobe_register(), update the callers, and kill
uprobe_register_refctr().
Signed-off-by: Oleg Nesterov
---
include/linux/uprobes.h | 9 ++---
kernel/events/uprobes.c | 23 +--
kernel/trace/bpf_trace.c| 2 +-
kernel/trace/trace_uprobe.c | 8 ++--
On 07/10, Oleg Nesterov wrote:
>
> Peter, these simple cleanups should not conflict with your changes,
> but I can resend them later if it causes any inconvenience.
In fact I would like to push 2 more cleanups before the more significant
changes, but they certainly conflict with your ong
On 06/30, Huacai Chen wrote:
>
> > +static __init int check_emit_break(void)
> > +{
> > + BUG_ON(UPROBE_SWBP_INSN != larch_insn_gen_break(BRK_UPROBE_BP));
> > + BUG_ON(UPROBE_XOLBP_INSN != larch_insn_gen_break(BRK_UPROBE_XOLBP));
> > + return 0;
> > +}
> > +arch_initcall(check_em
n Chancellor
Closes: https://lore.kernel.org/all/20240614174822.GA1185149@thelio-3990X/
Suggested-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
arch/loongarch/include/asm/inst.h| 3 +++
arch/loongarch/include/asm/uprobes.h | 4 ++--
arch/loongarch/kernel/kprobes.c | 12
On 06/29, Tiezhu Yang wrote:
>
> On Thu, 27 Jun 2024 19:38:06 +0200
> Oleg Nesterov wrote:
>
> ...
>
> > > > +arch_initcall(check_emit_break);
> > > > +
> > >
> > > I wouldn't even bother with this, but whatever.
> >
&g
On 06/27, Andrii Nakryiko wrote:
>
> Acked-by: Andrii Nakryiko
Thanks!
> > --- a/arch/loongarch/kernel/uprobes.c
> > +++ b/arch/loongarch/kernel/uprobes.c
> > @@ -7,6 +7,14 @@
> >
> > #define UPROBE_TRAP_NR UINT_MAX
> >
> > +static __init int check_emit_break(void)
> > +{
> > + BUG_ON(UPR
el.org/all/20240614174822.GA1185149@thelio-3990X/
Suggested-by: Andrii Nakryiko
Signed-off-by: Oleg Nesterov
---
arch/loongarch/include/asm/uprobes.h | 6 --
arch/loongarch/kernel/uprobes.c | 8
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/include/asm/up
On 06/27, Masami Hiramatsu wrote:
>
> On Thu, 27 Jun 2024 15:44:16 +0200
> Jiri Olsa wrote:
>
> > Oleg, do you want to send formal patch?
> >
> > thanks,
> > jirka
>
> Yes, can you send v2 patch?
I was waiting for the comments from loongarch maintainers...
OK, will do today, but the patch won't
On 06/20, Andrii Nakryiko wrote:
>
> On Thu, Jun 20, 2024 at 12:40 PM Oleg Nesterov wrote:
> >
> > But I can't understand what does it do, it calls emit_break() and
> > git grep -w emit_break finds nothing.
> >
>
> It's DEF_EMIT_REG0I15_FORMAT(br
On 06/20, Andrii Nakryiko wrote:
>
> Can we instead ask loongarch folks to rewrite it to be a constant?
> Having this as a function call is both an inconvenience and potential
> performance problem (a minor one, but still). I would imagine it's not
> hard to hard-code an instruction as a constant h
1 - 100 of 3003 matches
Mail list logo
