From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB2EF3B5846 for ; Tue, 16 Jun 2026 09:49:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781603365; cv=none; b=mU24HAbvSenMVVynEu8rihp+u/BFghWi8jvWneWquKbGZU344AVKU2Vx9CqwWP1tqsVRFnkRjQqlIIrNvq9SBt5RKFuIrAqbAqsvlTfZk5MQhJgBBmIRDXtcqsVcvPuHxV2nR4ItNmIKCWgsDQ8GUM6y+soklGohLnV1EM81Pvs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781603365; c=relaxed/simple; bh=hGCtVLzRbUgVfsb4xvTPGTUkavI1a1Af1ZRtb8cn8m8=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=Uk8XcBkH3vDqLSDh05o2aNrPL1pMWDHc9m8PGPuwR+ZfHOyinuwrHUA31WPcC1AU8Ddex4d2PgHMSnbsszwK8DYNxh0ODJRVsMGxi/JSnbTtvy2GfIGUcvpQH+OEVxVegsGg1faATK/ixtauYeoaJ/rAZATePDh8jP/q5cjqkHc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=STtVSgBY; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=Sd4jykWw; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="STtVSgBY"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="Sd4jykWw" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1781603362; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=iuOHnIXU71/pw1LKqPisCjflvP7Lf4r0qiAFUjk7OO8=; b=STtVSgBYyc9mc2hsB7SzKUKge8ui5dqskFhKUYBhU5tIEzn9HcjlH+GQbhiqBOOGuAZ3nR h+K/xWuoKnDAti6icQJDMArJnJEEc+D/hSQSZorykPxpDVolhHo0XHP6tlp1155fgFl6i+ vi3KStn6QIfe98At4OViHk0sMwkWo90= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-45-Nnk0WYAsPjaV2yE3DZSWXA-1; Tue, 16 Jun 2026 05:49:20 -0400 X-MC-Unique: Nnk0WYAsPjaV2yE3DZSWXA-1 X-Mimecast-MFC-AGG-ID: Nnk0WYAsPjaV2yE3DZSWXA_1781603359 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-461da6328c9so22718f8f.1 for ; Tue, 16 Jun 2026 02:49:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1781603359; x=1782208159; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=iuOHnIXU71/pw1LKqPisCjflvP7Lf4r0qiAFUjk7OO8=; b=Sd4jykWw6BuWWobtqXZKIdS38Rcqvws0oWHVJCOq4yTLrHTNAb71kjkedE7jali6p2 39HEiO3SapAGh7USdDQLSLffRjUm5gbHocYFrwX+Gu1sJkgRPsEesIzqwvETR5hOXxtm tF/B1c3goWXJQYqwSjNhCtf5R3oFjpP6+ZjU/xZK/M6zpHvGvSuY/sbOCNFmKdlXnqP4 g9NyfzmDdwnDSlB7R/tBbProIKQezl2IwVeZV316HIBgEOweXqXkfWyz1H4LJfWf0vIu MpYlhWjMuOfOL/xzOs04hbnROixyIQS5nOeYd9ey5wnPUBJYTzMDOZg0DYIitqZ8nIwI 1r9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781603359; x=1782208159; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=iuOHnIXU71/pw1LKqPisCjflvP7Lf4r0qiAFUjk7OO8=; b=YZGaiIEXOO8sNYyYqTrOpA6aXqxBANFZGsZZyPSAOoKuc9UryI/C2Qx0qShd6lo1eD U0ansgivaFQ90YEnpVYhtLozNoleEMv0S8a5f1bSKccEsJQ8SBI0yMSq9h9Ma810pjLF GKlVUOZCjXYzoma45BlpJW1ZWD85dooaU1/Rwy9AHI3evBUmCYoZpaHQmVkgbAGNnaYR HIw+4gmWeUwbQcq2Za2OOuKhBxT/7D0nWEBeJ89+wfHUK4GpqEx7IZwLqEmZ6ivlLnpm N1l2sHlzhxoC2+CTkNI2OTWFroAP4Ol0jsRRdY4M9FXb9f1B524YLydRLQjWYC0ZMWt5 Nrhw== X-Forwarded-Encrypted: i=1; AFNElJ8zrHrTN9k8iRWdsi3UHxr2cygHM8lUDkGutrICcoXYgzilGkVLTCHQuDF/0lKSI7lN8CfaEjwRXjdbAfs=@vger.kernel.org X-Gm-Message-State: AOJu0Yzd9CyYcCCF8dWrZQ16AwOZh8Nhmz+x+pjNgyfP82k9Xxdlvo9C 1PKBtQhUCwyM21EjjNf6CuuzgEXxaZWkSl+dbutnaj6/efvaG4sJ9QN+Zy1zzLVg140W47vQTzA dIkknBwiMN9Wp7cFf0yfOUjiey76iHtvWqY/FCUn7x3qpBrAfpUG6h9UDalRW3QpjdQ== X-Gm-Gg: Acq92OGob22KJhxgL7OsvvJDsTJAP0HiZsmakdhGawtW2jOdWuCBDH0EfxQo4reNY7y ZcNCqH1lNg8uuyI3XPfQ5Uby9+WZsEQzk4X/O+rdILQD5jRYU4ilPn4vNVhui8MtStAhjPEjAwU rm4cpD2txinptHHSG71MJCsTSoTb2s40fq1K6BAVrhz8sqOl6Qz+zWTaWAj3MY/Pn6MtUUf33lF NtHHvZsyDn81+5Bh/E+yE3ubVolvhbBSuybMKGYA09+6gAHR0WgaeSC2IOr7dZb7IUcZafJVGra Z1bphSHGucZKBSLl6HA6eiurtY4V88ro27w9ZOysS7proDCskafYfWTo0LrhM+yDoSCqMWaJt9j /HjNMgWLqeINsaUrN/xS6mMZxIQ== X-Received: by 2002:a5d:5d87:0:b0:460:11a7:f82e with SMTP id ffacd0b85a97d-4619f2e609bmr5459053f8f.8.1781603359259; Tue, 16 Jun 2026 02:49:19 -0700 (PDT) X-Received: by 2002:a5d:5d87:0:b0:460:11a7:f82e with SMTP id ffacd0b85a97d-4619f2e609bmr5458996f8f.8.1781603358714; Tue, 16 Jun 2026 02:49:18 -0700 (PDT) Received: from [192.168.1.167] ([185.168.96.228]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4606f26434dsm43387398f8f.1.2026.06.16.02.49.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jun 2026 02:49:18 -0700 (PDT) Message-ID: Subject: Re: [PATCH v3 2/9] rv: add generic uprobe infrastructure for RV monitors From: Gabriele Monaco To: wen.yang@linux.dev Cc: Steven Rostedt , linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org Date: Tue, 16 Jun 2026 11:49:17 +0200 In-Reply-To: <9d1a1d491af16853b2b421f358fd6cca965588ab.1780847473.git.wen.yang@linux.dev> References: <9d1a1d491af16853b2b421f358fd6cca965588ab.1780847473.git.wen.yang@linux.dev> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.2 (3.60.2-1.fc44) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Mon, 2026-06-08 at 00:13 +0800, wen.yang@linux.dev wrote: > From: Wen Yang >=20 > Introduce rv_uprobe, a thin wrapper around uprobe_consumer providing > rv_uprobe_attach_path(), rv_uprobe_attach(), and rv_uprobe_detach() > for RV monitors.=C2=A0 An opaque priv pointer is forwarded unchanged to > entry/return handlers so monitors can carry per-binding state (e.g. a > latency threshold) to the hot path without any global lookup. >=20 > rv_uprobe_detach() is fully synchronous (nosync + sync + path_put + > kfree), closing the use-after-free window present in open-coded > patterns where kfree() precedes uprobe_unregister_sync(). >=20 > Suggested-by: Gabriele Monaco > Signed-off-by: Wen Yang I find your implementation solid, but I wonder if it isn't a bit overdoing it. It's good to have helper functions abstracting away the common uprobes code, but I find the number of structures used to indirect/h= ide the process a bit confusing. Users of this headers still know they're using uprobes, so we don't really need to hide it that much, plus the double function pointers are an additional performance hit we could easily skip. As I get it, we want to pass a private pointer (essentially to get the threshold in tlob) and the uprobe infrastructure doesn't allow that transparently. So let's keep on using struct embedding but a bit more cut to the bone: struct rv_uprobe { struct uprobe_consumer uc; struct uprobe *uprobe; void *priv; }; Then handlers would use container_of() appropriately to derive the data the= y need. static int tlob_uprobe_entry_handler(struct uprobe_consumer *self, struct = pt_regs *regs, __u64 *data) { struct rv_uprobe *p =3D container_of(self, struct rv_uprobe, uc); struct tlob_uprobe_binding *b =3D p->priv; ... } You could even define the struct directly inside your private data and save= one step (and no void *): struct rv_uprobe { struct uprobe_consumer uc; struct uprobe *uprobe; }; #define DECLARE_RV_UPROBE(name) struct rv_uprobe name // And then struct tlob_uprobe_binding { u64 threshold_ns; ... DECLARE_RV_UPROBE(start); DECLARE_RV_UPROBE(stop); } static int tlob_uprobe_entry_handler(struct uprobe_consumer *self, struct = pt_regs *regs, __u64 *data) { struct tlob_uprobe_binding *b =3D container_of(self, struct tlob_uprobe_b= inding, start.uc); ... } Now I intentionally skipped offset, as I don't really see why carry it arou= nd, and path, as we could live without it by relying on inodes (uprobes reference-track them but you'd need to save them somewhere). Mind, I only build tested this idea (appended after). This was supported by= an LLM which did a few more changes like standardising names and return values= of registration functions (a bit more consistent with other stuff, you don't h= ave to commit with that though). By the way, do we really need the duplicate check? Would it break if a user defines it twice? If not, we could simplify the tlob_add_uprobe() even furt= her and doing all path related checks in the library, since those aren't quite = tlob specific. Thoughts? Thanks, Gabriele **Suggested simplification:** --- diff --git a/include/rv/rv_uprobe.h b/include/rv/rv_uprobe.h index 9106c5c927..4fb3f50a63 100644 --- a/include/rv/rv_uprobe.h +++ b/include/rv/rv_uprobe.h @@ -7,83 +7,56 @@ #ifndef _RV_UPROBE_H #define _RV_UPROBE_H =20 -#include #include +#include =20 struct pt_regs; +struct inode; =20 /** * struct rv_uprobe - a single uprobe registered on behalf of an RV monito= r * - * @offset: byte offset within the ELF binary where the probe is install= ed - * @priv: monitor-private pointer; set at attach time, never touched b= y - * this layer; passed unchanged to entry_fn / ret_fn - * @path: resolved path of the probed binary (read-only after attach); - * callers may use path.dentry for identity comparisons - * - * The implementation fields (uprobe_consumer, uprobe handle, callbacks) a= re - * private to rv_uprobe.c and are not exposed here; monitors must not acce= ss - * them directly. + * @uc: underlying uprobe consumer (publicly visible) + * @uprobe: active uprobe structure handle + * @inode: inode of the target binary (read-only after registration) */ struct rv_uprobe { - /* public: read-only after rv_uprobe_attach*() */ - loff_t offset; - void *priv; - struct path path; + struct uprobe_consumer uc; + struct uprobe *uprobe; + struct inode *inode; }; =20 -/** - * rv_uprobe_attach_path - register an uprobe given an already-resolved pa= th - * @path: path of the target binary; rv_uprobe takes its own reference - * @offset: byte offset within the binary - * @entry_fn: called on probe hit (entry); may be NULL - * @ret_fn: called on function return (uretprobe); may be NULL - * @priv: opaque pointer forwarded to callbacks unchanged - * - * Use this variant when the caller has already resolved the path (e.g. to - * register multiple probes on the same binary with a single kern_path cal= l). - * The inode is derived internally via d_real_inode(), so inode and path a= re - * always consistent. - * - * Returns a pointer to the new rv_uprobe on success, ERR_PTR on failure. - */ -struct rv_uprobe *rv_uprobe_attach_path(struct path *path, loff_t offset, - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data), - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data), - void *priv); +/* Seamless inline declaration of a named uprobe inside user structs */ +#define DECLARE_RV_UPROBE(name) \ + struct rv_uprobe name =20 /** - * rv_uprobe_attach - resolve binpath and register an uprobe - * @binpath: absolute path to the target binary - * @offset: byte offset within the binary - * @entry_fn: called on probe hit (entry); may be NULL - * @ret_fn: called on function return (uretprobe); may be NULL - * @priv: opaque pointer forwarded to callbacks unchanged + * rv_uprobe_register - resolve binpath and register an uprobe + * @binpath: absolute path to the target binary + * @offset: byte offset within the binary + * @p: pointer to the allocated/embedded rv_uprobe structure + * @handler: called on probe hit (entry); may be NULL + * @ret_handler: called on function return (uretprobe); may be NULL * - * Resolves binpath via kern_path(), then delegates to rv_uprobe_attach_pa= th(). + * Resolves binpath via kern_path(), registers the uprobe directly using t= he + * embedded `uprobe_consumer`, and immediately releases the path reference= . * - * Returns a pointer to the new rv_uprobe on success, ERR_PTR on failure. + * Returns 0 on success, or a negative error code on failure. */ -struct rv_uprobe *rv_uprobe_attach(const char *binpath, loff_t offset, - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data), - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data), - void *priv); +int rv_uprobe_register(const char *binpath, loff_t offset, struct rv_uprob= e *p, + int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs, __u64 = *data), + int (*ret_handler)(struct uprobe_consumer *self, unsigned long func, + struct pt_regs *regs, __u64 *data)); =20 /** - * rv_uprobe_detach - synchronously unregister an uprobe and free it - * @p: probe to detach; may be NULL (no-op) - * - * Calls uprobe_unregister_nosync(), then uprobe_unregister_sync() to wait - * for any in-progress handler to finish, then releases the path reference - * and frees the rv_uprobe struct. The caller's priv data is NOT freed. + * rv_uprobe_unregister - synchronously unregister a uprobe + * @p: probe to unregister; may be NULL (no-op) * - * When removing a single probe, prefer this over the three-phase API. - * Safe to call from process context only (uprobe_unregister_sync() may - * schedule). + * Dequeues the uprobe and waits synchronously for all in-flight handlers + * to complete. */ -void rv_uprobe_detach(struct rv_uprobe *p); +void rv_uprobe_unregister(struct rv_uprobe *p); =20 /** * rv_uprobe_unregister_nosync - dequeue an uprobe without waiting @@ -91,9 +64,7 @@ void rv_uprobe_detach(struct rv_uprobe *p); * * Removes the uprobe from the uprobe subsystem but does NOT wait for * in-flight handlers to complete. The caller must call rv_uprobe_sync() - * before calling rv_uprobe_free() on the same probe. - * - * Use this to batch multiple deregistrations before a single rv_uprobe_sy= nc(). + * before freeing any container holding this probe. */ void rv_uprobe_unregister_nosync(struct rv_uprobe *p); =20 @@ -101,19 +72,8 @@ void rv_uprobe_unregister_nosync(struct rv_uprobe *p); * rv_uprobe_sync - wait for all in-flight uprobe handlers to complete * * Global barrier: waits for every in-flight uprobe handler across the sys= tem - * to finish. Call once after a batch of rv_uprobe_unregister_nosync() ca= lls - * and before any rv_uprobe_free() call. + * to finish. */ void rv_uprobe_sync(void); =20 -/** - * rv_uprobe_free - release resources of a previously deregistered probe - * @p: probe to free; may be NULL (no-op) - * - * Releases the path reference and frees the rv_uprobe struct. Must only - * be called after rv_uprobe_sync() has returned. The caller's priv data - * is NOT freed. - */ -void rv_uprobe_free(struct rv_uprobe *p); - #endif /* _RV_UPROBE_H */ diff --git a/kernel/trace/rv/monitors/tlob/tlob.c b/kernel/trace/rv/monitor= s/tlob/tlob.c index d8e0c47947..28a6c740c7 100644 --- a/kernel/trace/rv/monitors/tlob/tlob.c +++ b/kernel/trace/rv/monitors/tlob/tlob.c @@ -252,8 +252,8 @@ struct tlob_uprobe_binding { char binpath[TLOB_MAX_PATH]; loff_t offset_start; loff_t offset_stop; - struct rv_uprobe *start_probe; - struct rv_uprobe *stop_probe; + DECLARE_RV_UPROBE(start_probe); + DECLARE_RV_UPROBE(stop_probe); }; =20 /* RCU callback: free the slab once no readers remain. */ @@ -512,16 +512,16 @@ int tlob_stop_task(struct task_struct *task) EXPORT_SYMBOL_GPL(tlob_stop_task); =20 =20 -static int tlob_uprobe_entry_handler(struct rv_uprobe *p, struct pt_regs *= regs, +static int tlob_uprobe_entry_handler(struct uprobe_consumer *self, struct = pt_regs *regs, __u64 *data) { - struct tlob_uprobe_binding *b =3D p->priv; + struct tlob_uprobe_binding *b =3D container_of(self, struct tlob_uprobe_b= inding, start_probe.uc); =20 tlob_start_task(current, b->threshold_ns); return 0; } =20 -static int tlob_uprobe_stop_handler(struct rv_uprobe *p, struct pt_regs *r= egs, +static int tlob_uprobe_stop_handler(struct uprobe_consumer *self, struct p= t_regs *regs, __u64 *data) { tlob_stop_task(current); @@ -537,6 +537,7 @@ static int tlob_add_uprobe(u64 threshold_ns, const char= *binpath, { struct tlob_uprobe_binding *b, *tmp_b; char pathbuf[TLOB_MAX_PATH]; + struct inode *inode; struct path path; char *canon; int ret; @@ -561,10 +562,12 @@ static int tlob_add_uprobe(u64 threshold_ns, const ch= ar *binpath, goto err_path; } =20 - /* Reject duplicate start offset for the same binary. */ + inode =3D d_real_inode(path.dentry); + + /* Reject duplicate start offset for the same binary inode. */ list_for_each_entry(tmp_b, &tlob_uprobe_list, list) { if (tmp_b->offset_start =3D=3D offset_start && - tmp_b->start_probe->path.dentry =3D=3D path.dentry) { + tmp_b->start_probe.inode =3D=3D inode) { ret =3D -EEXIST; goto err_path; } @@ -577,29 +580,25 @@ static int tlob_add_uprobe(u64 threshold_ns, const ch= ar *binpath, } strscpy(b->binpath, canon, sizeof(b->binpath)); =20 - /* Both probes share b (priv) and path; attach_path refs path itself. */ - b->start_probe =3D rv_uprobe_attach_path(&path, offset_start, - tlob_uprobe_entry_handler, NULL, b); - if (IS_ERR(b->start_probe)) { - ret =3D PTR_ERR(b->start_probe); - b->start_probe =3D NULL; - goto err_path; - } + path_put(&path); + + /* Both probes are registered directly on the embedded fields */ + ret =3D rv_uprobe_register(b->binpath, offset_start, &b->start_probe, + tlob_uprobe_entry_handler, NULL); + if (ret) + goto err_free; =20 - b->stop_probe =3D rv_uprobe_attach_path(&path, offset_stop, - tlob_uprobe_stop_handler, NULL, b); - if (IS_ERR(b->stop_probe)) { - ret =3D PTR_ERR(b->stop_probe); - b->stop_probe =3D NULL; + ret =3D rv_uprobe_register(b->binpath, offset_stop, &b->stop_probe, + tlob_uprobe_stop_handler, NULL); + if (ret) goto err_start; - } =20 - path_put(&path); list_add_tail(&b->list, &tlob_uprobe_list); return 0; =20 err_start: - rv_uprobe_detach(b->start_probe); + rv_uprobe_unregister(&b->start_probe); + goto err_free; err_path: path_put(&path); err_free: @@ -611,21 +610,24 @@ static int tlob_remove_uprobe_by_key(loff_t offset_st= art, const char *binpath) { struct tlob_uprobe_binding *b, *tmp; struct path remove_path; + struct inode *inode; int ret; =20 ret =3D kern_path(binpath, LOOKUP_FOLLOW, &remove_path); if (ret) return ret; =20 + inode =3D d_real_inode(remove_path.dentry); + ret =3D -ENOENT; list_for_each_entry_safe(b, tmp, &tlob_uprobe_list, list) { if (b->offset_start !=3D offset_start) continue; - if (b->start_probe->path.dentry !=3D remove_path.dentry) + if (b->start_probe.inode !=3D inode) continue; list_del(&b->list); - rv_uprobe_detach(b->start_probe); - rv_uprobe_detach(b->stop_probe); + rv_uprobe_unregister(&b->start_probe); + rv_uprobe_unregister(&b->stop_probe); kfree(b); ret =3D 0; break; @@ -643,8 +645,8 @@ static void tlob_remove_all_uprobes(void) mutex_lock(&tlob_uprobe_mutex); list_for_each_entry_safe(b, tmp, &tlob_uprobe_list, list) { list_move(&b->list, &pending); - rv_uprobe_unregister_nosync(b->start_probe); - rv_uprobe_unregister_nosync(b->stop_probe); + rv_uprobe_unregister_nosync(&b->start_probe); + rv_uprobe_unregister_nosync(&b->stop_probe); } mutex_unlock(&tlob_uprobe_mutex); =20 @@ -658,8 +660,6 @@ static void tlob_remove_all_uprobes(void) rv_uprobe_sync(); =20 list_for_each_entry_safe(b, tmp, &pending, list) { - rv_uprobe_free(b->start_probe); - rv_uprobe_free(b->stop_probe); kfree(b); } } diff --git a/kernel/trace/rv/rv_uprobe.c b/kernel/trace/rv/rv_uprobe.c index 3d8b764dde..69b8b0c27e 100644 --- a/kernel/trace/rv/rv_uprobe.c +++ b/kernel/trace/rv/rv_uprobe.c @@ -10,149 +10,74 @@ #include #include =20 -/* - * Private extension of struct rv_uprobe. Allocated by rv_uprobe_attach*(= ) - * and returned to callers as &impl->pub. - */ -struct rv_uprobe_impl { - struct rv_uprobe pub; /* must be first; callers hold &pub */ - struct uprobe_consumer uc; - struct uprobe *uprobe; - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data); - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data); -}; - -static int rv_uprobe_handler(struct uprobe_consumer *uc, - struct pt_regs *regs, __u64 *data) -{ - struct rv_uprobe_impl *impl =3D container_of(uc, struct rv_uprobe_impl, u= c); - - if (impl->entry_fn) - return impl->entry_fn(&impl->pub, regs, data); - return 0; -} - -static int rv_uprobe_ret_handler(struct uprobe_consumer *uc, - unsigned long func, - struct pt_regs *regs, __u64 *data) -{ - struct rv_uprobe_impl *impl =3D container_of(uc, struct rv_uprobe_impl, u= c); - - if (impl->ret_fn) - return impl->ret_fn(&impl->pub, func, regs, data); - return 0; -} - -static struct rv_uprobe * -__rv_uprobe_attach(struct inode *inode, struct path *path, loff_t offset, - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *dat= a), - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data), - void *priv) -{ - struct rv_uprobe_impl *impl; - int ret; - - if (!entry_fn && !ret_fn) - return ERR_PTR(-EINVAL); - - impl =3D kzalloc_obj(*impl, GFP_KERNEL); - if (!impl) - return ERR_PTR(-ENOMEM); - - impl->pub.offset =3D offset; - impl->pub.priv =3D priv; - impl->entry_fn =3D entry_fn; - impl->ret_fn =3D ret_fn; - path_get(path); - impl->pub.path =3D *path; - - if (entry_fn) - impl->uc.handler =3D rv_uprobe_handler; - if (ret_fn) - impl->uc.ret_handler =3D rv_uprobe_ret_handler; - - impl->uprobe =3D uprobe_register(inode, offset, 0, &impl->uc); - if (IS_ERR(impl->uprobe)) { - ret =3D PTR_ERR(impl->uprobe); - path_put(&impl->pub.path); - kfree(impl); - return ERR_PTR(ret); - } - - return &impl->pub; -} - -/** - * rv_uprobe_attach_path - register an uprobe given an already-resolved pa= th - */ -struct rv_uprobe *rv_uprobe_attach_path(struct path *path, loff_t offset, - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data), - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data), - void *priv) -{ - struct inode *inode =3D d_real_inode(path->dentry); - - return __rv_uprobe_attach(inode, path, offset, entry_fn, ret_fn, priv); -} -EXPORT_SYMBOL_GPL(rv_uprobe_attach_path); - /** - * rv_uprobe_attach - resolve binpath and register an uprobe + * rv_uprobe_register - resolve binpath and register an uprobe */ -struct rv_uprobe *rv_uprobe_attach(const char *binpath, loff_t offset, - int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data), - int (*ret_fn)(struct rv_uprobe *p, unsigned long func, - struct pt_regs *regs, __u64 *data), - void *priv) +int rv_uprobe_register(const char *binpath, loff_t offset, struct rv_uprob= e *p, + int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs, __u64 = *data), + int (*ret_handler)(struct uprobe_consumer *self, unsigned long func, + struct pt_regs *regs, __u64 *data)) { - struct rv_uprobe *p; + struct inode *inode; struct path path; int ret; =20 + if (!handler && !ret_handler) + return -EINVAL; + ret =3D kern_path(binpath, LOOKUP_FOLLOW, &path); if (ret) - return ERR_PTR(ret); + return ret; =20 if (!d_is_reg(path.dentry)) { path_put(&path); - return ERR_PTR(-EINVAL); + return -EINVAL; } =20 - p =3D rv_uprobe_attach_path(&path, offset, entry_fn, ret_fn, priv); + inode =3D d_real_inode(path.dentry); + + p->uc.handler =3D handler; + p->uc.ret_handler =3D ret_handler; + p->inode =3D inode; + + p->uprobe =3D uprobe_register(inode, offset, 0, &p->uc); path_put(&path); - return p; + + if (IS_ERR(p->uprobe)) { + ret =3D PTR_ERR(p->uprobe); + p->uprobe =3D NULL; + p->inode =3D NULL; + return ret; + } + + return 0; } -EXPORT_SYMBOL_GPL(rv_uprobe_attach); +EXPORT_SYMBOL_GPL(rv_uprobe_register); =20 /** - * rv_uprobe_detach - synchronously unregister an uprobe and free it + * rv_uprobe_unregister - synchronously unregister a uprobe */ -void rv_uprobe_detach(struct rv_uprobe *p) +void rv_uprobe_unregister(struct rv_uprobe *p) { - if (!p) + if (!p || IS_ERR_OR_NULL(p->uprobe)) return; =20 rv_uprobe_unregister_nosync(p); rv_uprobe_sync(); - rv_uprobe_free(p); } -EXPORT_SYMBOL_GPL(rv_uprobe_detach); +EXPORT_SYMBOL_GPL(rv_uprobe_unregister); =20 /** * rv_uprobe_unregister_nosync - dequeue an uprobe without waiting */ void rv_uprobe_unregister_nosync(struct rv_uprobe *p) { - struct rv_uprobe_impl *impl; - - if (!p) + if (!p || IS_ERR_OR_NULL(p->uprobe)) return; =20 - impl =3D container_of(p, struct rv_uprobe_impl, pub); - uprobe_unregister_nosync(impl->uprobe, &impl->uc); + uprobe_unregister_nosync(p->uprobe, &p->uc); + p->uprobe =3D NULL; + p->inode =3D NULL; } EXPORT_SYMBOL_GPL(rv_uprobe_unregister_nosync); =20 @@ -164,19 +89,3 @@ void rv_uprobe_sync(void) uprobe_unregister_sync(); } EXPORT_SYMBOL_GPL(rv_uprobe_sync); - -/** - * rv_uprobe_free - release resources of a previously deregistered probe - */ -void rv_uprobe_free(struct rv_uprobe *p) -{ - struct rv_uprobe_impl *impl; - - if (!p) - return; - - impl =3D container_of(p, struct rv_uprobe_impl, pub); - path_put(&p->path); - kfree(impl); -} -EXPORT_SYMBOL_GPL(rv_uprobe_free);