From nobody Mon Apr 6 14:55:30 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3D24C433F5 for ; Thu, 29 Sep 2022 22:56:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230220AbiI2W4P (ORCPT ); Thu, 29 Sep 2022 18:56:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48722 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229606AbiI2Wzf (ORCPT ); Thu, 29 Sep 2022 18:55:35 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8B23CD74D6 for ; Thu, 29 Sep 2022 15:55:28 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 8FAE9621C2 for ; Thu, 29 Sep 2022 22:55:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0A399C43142; Thu, 29 Sep 2022 22:55:27 +0000 (UTC) Received: from rostedt by gandalf.local.home with local (Exim 4.96) (envelope-from ) id 1oe2SO-000csD-2u; Thu, 29 Sep 2022 18:56:40 -0400 Message-ID: <20220929225640.475558980@goodmis.org> User-Agent: quilt/0.66 Date: Thu, 29 Sep 2022 18:55:55 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org Cc: Ingo Molnar , Andrew Morton , Mathieu Desnoyers , Beau Belgrave Subject: [for-next][PATCH 13/15] tracing/user_events: Use refcount instead of atomic for ref tracking References: <20220929225542.784716766@goodmis.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Beau Belgrave User processes could open up enough event references to cause rollovers. These could cause use after free scenarios, which we do not want. Switching to refcount APIs prevent this, but will leak memory once saturated. Once saturated, user processes can still use the events. This prevents a bad user process from stopping existing telemetry from being emitted. Link: https://lkml.kernel.org/r/20220728233309.1896-5-beaub@linux.microsoft= .com Link: https://lore.kernel.org/all/2059213643.196683.1648499088753.JavaMail.= zimbra@efficios.com/ Reported-by: Mathieu Desnoyers Signed-off-by: Beau Belgrave Signed-off-by: Steven Rostedt (Google) --- kernel/trace/trace_events_user.c | 53 +++++++++++++++----------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_u= ser.c index f9bb7d37d76f..2bcae7abfa81 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -57,7 +58,7 @@ static DECLARE_BITMAP(page_bitmap, MAX_EVENTS); * within a file a user_event might be created if it does not * already exist. These are globally used and their lifetime * is tied to the refcnt member. These cannot go away until the - * refcnt reaches zero. + * refcnt reaches one. */ struct user_event { struct tracepoint tracepoint; @@ -67,7 +68,7 @@ struct user_event { struct hlist_node node; struct list_head fields; struct list_head validators; - atomic_t refcnt; + refcount_t refcnt; int index; int flags; int min_size; @@ -105,6 +106,12 @@ static u32 user_event_key(char *name) return jhash(name, strlen(name), 0); } =20 +static __always_inline __must_check +bool user_event_last_ref(struct user_event *user) +{ + return refcount_read(&user->refcnt) =3D=3D 1; +} + static __always_inline __must_check size_t copy_nofault(void *addr, size_t bytes, struct iov_iter *i) { @@ -662,7 +669,7 @@ static struct user_event *find_user_event(char *name, u= 32 *outkey) =20 hash_for_each_possible(register_table, user, node, key) if (!strcmp(EVENT_NAME(user), name)) { - atomic_inc(&user->refcnt); + refcount_inc(&user->refcnt); return user; } =20 @@ -876,12 +883,12 @@ static int user_event_reg(struct trace_event_call *ca= ll, =20 return ret; inc: - atomic_inc(&user->refcnt); + refcount_inc(&user->refcnt); update_reg_page_for(user); return 0; dec: update_reg_page_for(user); - atomic_dec(&user->refcnt); + refcount_dec(&user->refcnt); return 0; } =20 @@ -907,7 +914,7 @@ static int user_event_create(const char *raw_command) ret =3D user_event_parse_cmd(name, &user); =20 if (!ret) - atomic_dec(&user->refcnt); + refcount_dec(&user->refcnt); =20 mutex_unlock(®_mutex); =20 @@ -951,14 +958,14 @@ static bool user_event_is_busy(struct dyn_event *ev) { struct user_event *user =3D container_of(ev, struct user_event, devent); =20 - return atomic_read(&user->refcnt) !=3D 0; + return !user_event_last_ref(user); } =20 static int user_event_free(struct dyn_event *ev) { struct user_event *user =3D container_of(ev, struct user_event, devent); =20 - if (atomic_read(&user->refcnt) !=3D 0) + if (!user_event_last_ref(user)) return -EBUSY; =20 return destroy_user_event(user); @@ -1137,8 +1144,8 @@ static int user_event_parse(char *name, char *args, c= har *flags, =20 user->index =3D index; =20 - /* Ensure we track ref */ - atomic_inc(&user->refcnt); + /* Ensure we track self ref and caller ref (2) */ + refcount_set(&user->refcnt, 2); =20 dyn_event_init(&user->devent, &user_event_dops); dyn_event_add(&user->devent, &user->call); @@ -1164,29 +1171,17 @@ static int user_event_parse(char *name, char *args,= char *flags, static int delete_user_event(char *name) { u32 key; - int ret; struct user_event *user =3D find_user_event(name, &key); =20 if (!user) return -ENOENT; =20 - /* Ensure we are the last ref */ - if (atomic_read(&user->refcnt) !=3D 1) { - ret =3D -EBUSY; - goto put_ref; - } - - ret =3D destroy_user_event(user); + refcount_dec(&user->refcnt); =20 - if (ret) - goto put_ref; - - return ret; -put_ref: - /* No longer have this ref */ - atomic_dec(&user->refcnt); + if (!user_event_last_ref(user)) + return -EBUSY; =20 - return ret; + return destroy_user_event(user); } =20 /* @@ -1314,7 +1309,7 @@ static int user_events_ref_add(struct file *file, str= uct user_event *user) =20 new_refs->events[i] =3D user; =20 - atomic_inc(&user->refcnt); + refcount_inc(&user->refcnt); =20 rcu_assign_pointer(file->private_data, new_refs); =20 @@ -1374,7 +1369,7 @@ static long user_events_ioctl_reg(struct file *file, = unsigned long uarg) ret =3D user_events_ref_add(file, user); =20 /* No longer need parse ref, ref_add either worked or not */ - atomic_dec(&user->refcnt); + refcount_dec(&user->refcnt); =20 /* Positive number is index and valid */ if (ret < 0) @@ -1464,7 +1459,7 @@ static int user_events_release(struct inode *node, st= ruct file *file) user =3D refs->events[i]; =20 if (user) - atomic_dec(&user->refcnt); + refcount_dec(&user->refcnt); } out: file->private_data =3D NULL; --=20 2.35.1