On Tue, Jul 30, 2024 at 5:34 AM Oleg Nesterov <[email protected]> wrote: > > If register_for_each_vma() fails uprobe_register() can safely drop > uprobe->register_rwsem and use uprobe_unregister(). There is no worry > about the races with another register/unregister, consumer_add() was > already called so this case doesn't differ from _unregister() right > after the successful _register(). > > Yes this means the extra up_write() + down_write(), but this is the > slow and unlikely case anyway. > > Signed-off-by: Oleg Nesterov <[email protected]> > --- > kernel/events/uprobes.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) >
Yep, makes total sense, in my local patches I basically already done that as well. Acked-by: Andrii Nakryiko <[email protected]> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 974474680820..5ea0aabe8774 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -1174,16 +1174,18 @@ struct uprobe *uprobe_register(struct inode *inode, > if (likely(uprobe_is_active(uprobe))) { > consumer_add(uprobe, uc); > ret = register_for_each_vma(uprobe, uc); > - if (ret) > - __uprobe_unregister(uprobe, uc); > } > up_write(&uprobe->register_rwsem); > put_uprobe(uprobe); > > - if (unlikely(ret == -EAGAIN)) > - goto retry; > + if (ret) { > + if (unlikely(ret == -EAGAIN)) > + goto retry; > + uprobe_unregister(uprobe, uc); > + return ERR_PTR(ret); > + } > > - return ret ? ERR_PTR(ret) : uprobe; > + return uprobe; > } > EXPORT_SYMBOL_GPL(uprobe_register); > > -- > 2.25.1.362.g51ebf55 >
