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 {
> - err = register_for_each_vma(uprobe, NULL);
> - /* TODO : cant unregister? schedule a worker thread */
> - if (unlikely(err))
> - uprobe_warn(current, "unregister, leaking uprobe");
> - }
> +
> + list_del_rcu(&uc->cons_node);
> + err = register_for_each_vma(uprobe, NULL);
> +
> up_write(&uprobe->register_rwsem);
>
> - if (!err)
> - put_uprobe(uprobe);
> + /* TODO : cant unregister? schedule a worker thread */
> + if (unlikely(err)) {
> + uprobe_warn(current, "unregister, leaking uprobe");
> + return;
Looks wrong... We can (should) skip put_uprobe(), but we can't avoid
synchronize_srcu().
The caller can free the consumer right after return. You even added
a fat comment below.
Yes, the problem will go away after you split it into nosync/sync, but
still.
Oleg.
