On Wed, May 14, 2025 at 12:18:09PM +0200, Jiri Olsa wrote: > From: Jiri Olsa <[email protected]> > > There's error path that could lead to inactive uprobe: > > 1) uprobe_register succeeds - updates instruction to int3 and > changes ref_ctr from 0 to 1 > 2) uprobe_unregister fails - int3 stays in place, but ref_ctr > is changed to 0 (it's not restored to 1 in the fail path) > uprobe is leaked > 3) another uprobe_register comes and re-uses the leaked uprobe > and succeds - but int3 is already in place, so ref_ctr update > is skipped and it stays 0 - uprobe CAN NOT be triggered now > 4) uprobe_unregister fails because ref_ctr value is unexpected > > Fixing this by reverting the updated ref_ctr value back to 1 in step 2), > which is the case when uprobe_unregister fails (int3 stays in place), > but we have already updated refctr. > > The new scenario will go as follows: > > 1) uprobe_register succeeds - updates instruction to int3 and > changes ref_ctr from 0 to 1 > 2) uprobe_unregister fails - int3 stays in place and ref_ctr > is reverted to 1.. uprobe is leaked > 3) another uprobe_register comes and re-uses the leaked uprobe > and succeds - but int3 is already in place, so ref_ctr update > is skipped and it stays 1 - uprobe CAN be triggered now > 4) uprobe_unregister succeeds > > Fixes: 1cc33161a83d ("uprobes: Support SDT markers having reference count > (semaphore)") > Acked-by: David Hildenbrand <[email protected]> > Acked-by: Oleg Nesterov <[email protected]> > Suggested-by: Oleg Nesterov <[email protected]> > Signed-off-by: Jiri Olsa <[email protected]>
hi, I can't find this in any related tree, was this pulled in? thanks, jirka > --- > v2 changes: > - adding proper Fixes tag and acks > > kernel/events/uprobes.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 4c965ba77f9f..84ee7b590861 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -581,8 +581,8 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, > struct vm_area_struct *vma, > > out: > /* Revert back reference counter if instruction update failed. */ > - if (ret < 0 && is_register && ref_ctr_updated) > - update_ref_ctr(uprobe, mm, -1); > + if (ret < 0 && ref_ctr_updated) > + update_ref_ctr(uprobe, mm, is_register ? -1 : 1); > > /* try collapse pmd for compound page */ > if (ret > 0) > -- > 2.49.0 >
