From nobody Tue Nov 26 22:37:47 2024 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 5ABFD147C91 for ; Tue, 15 Oct 2024 15:31:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729006266; cv=none; b=hIbrDoU8iTinuOH04oNalKlOOoyOLx+kkVLboZdNFH1i9h5tRY7q/nV0LOeYToA/Om8fWYvtuXPQBuvRXEzMQ8duMh2EyT5yHmtgtdpAb2q0vTtSCSdtWEViuaGDK+BmCl1be62Z0OUsLy5BEfQh08R2Zl1poFtUcOD3JTnXcb8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729006266; c=relaxed/simple; bh=i59obFdqyI72PyNeSYD1U0n4WunXxK1/6SIvjpa+z/Q=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=BvY1kVuW/R068KvlEAiUUWKub4+Jy7GQSc+eDV4Ae4hk9o/OvFLS75rEllMBGiBtO+J9W6rE/gUgmwLAXeXJPgZB1kypUr/vWjDhiRt3hSUmNx2W/pZwlAOIUHvoo/astM5YOoOvGlMGM0mR2zNM717Gqx5K9ynYYMix571/zmA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 10452C4CECF; Tue, 15 Oct 2024 15:31:06 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.98) (envelope-from ) id 1t0jW9-00000002z4s-2ejC; Tue, 15 Oct 2024 11:31:25 -0400 Message-ID: <20241015153125.494414250@goodmis.org> User-Agent: quilt/0.68 Date: Tue, 15 Oct 2024 11:31:06 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton Subject: [for-next][PATCH 1/2] ring-buffer: Fix refcount setting of boot mapped buffers References: <20241015153105.843619901@goodmis.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Steven Rostedt A ring buffer which has its buffered mapped at boot up to fixed memory should not be freed. Other buffers can be. The ref counting setup was wrong for both. It made the not mapped buffers ref count have zero, and the boot mapped buffer a ref count of 1. But an normally allocated buffer should be 1, where it can be removed. Keep the ref count of a normal boot buffer with its setup ref count (do not decrement it), and increment the fixed memory boot mapped buffer's ref count. Cc: Mathieu Desnoyers Link: https://lore.kernel.org/20241011165224.33dd2624@gandalf.local.home Fixes: e645535a954ad ("tracing: Add option to use memmapped memory for trac= e boot instance") Reviewed-by: Masami Hiramatsu (Google) Signed-off-by: Steven Rostedt (Google) --- kernel/trace/trace.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 1c69ca1f1088..a8f52b6527ca 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -10621,10 +10621,10 @@ __init static void enable_instances(void) * cannot be deleted by user space, so keep the reference * to it. */ - if (start) + if (start) { tr->flags |=3D TRACE_ARRAY_FL_BOOT; - else - trace_array_put(tr); + tr->ref++; + } =20 while ((tok =3D strsep(&curr_str, ","))) { early_enable_events(tr, tok, true); --=20 2.45.2 From nobody Tue Nov 26 22:37:47 2024 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 B3A4B1D8A12 for ; Tue, 15 Oct 2024 15:31:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729006266; cv=none; b=qnUh/eR94NuvgnbT84fAveDOIE4dXucAnjycCfJQWZKT5OrqbDhq5vyDZgTiA7IyL22ooGQetCQaT6ik3vA4p2gcM4ZS1GiMZYdq0WlKGkke5k3BotQRM0nNq4EeTXIv/zcky7LwzPHHjmUSJ8uEUUTyN9l2UIluNEnDn1R6QfE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729006266; c=relaxed/simple; bh=SzgDNqXDB3VwKZFe2irsr0kmfNOEzg1eE1sTlNR1tbc=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=MCmhNQ4Yccfn23VkUbz9RvUUVfS9AD8skVJJodr5JGVCENI7OZo4VG/1MZcSEKF3vbcB3iRBluna8STpIzawS6TV0pBRuUz12ytSmSmFKVHl2C3LhG4nPvt5mX1wm5y3avGXk8kZ0t643pbBqIX/7GxHbGTxtvZhuOpg0/7brzI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 62EE1C4CED0; Tue, 15 Oct 2024 15:31:06 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.98) (envelope-from ) id 1t0jW9-00000002z5N-3JiT; Tue, 15 Oct 2024 11:31:25 -0400 Message-ID: <20241015153125.652461107@goodmis.org> User-Agent: quilt/0.68 Date: Tue, 15 Oct 2024 11:31:07 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , Petr Pavlu Subject: [for-next][PATCH 2/2] ring-buffer: Fix reader locking when changing the sub buffer order References: <20241015153105.843619901@goodmis.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Petr Pavlu The function ring_buffer_subbuf_order_set() updates each ring_buffer_per_cpu and installs new sub buffers that match the requested page order. This operation may be invoked concurrently with readers that rely on some of the modified data, such as the head bit (RB_PAGE_HEAD), or the ring_buffer_per_cpu.pages and reader_page pointers. However, no exclusive access is acquired by ring_buffer_subbuf_order_set(). Modifying the mentioned data while a reader also operates on them can then result in incorrect memory access and various crashes. Fix the problem by taking the reader_lock when updating a specific ring_buffer_per_cpu in ring_buffer_subbuf_order_set(). Link: https://lore.kernel.org/linux-trace-kernel/20240715145141.5528-1-petr= .pavlu@suse.com/ Link: https://lore.kernel.org/linux-trace-kernel/20241010195849.2f77cc3f@ga= ndalf.local.home/ Link: https://lore.kernel.org/linux-trace-kernel/20241011112850.17212b25@ga= ndalf.local.home/ Cc: Masami Hiramatsu Cc: Mathieu Desnoyers Link: https://lore.kernel.org/20241015112440.26987-1-petr.pavlu@suse.com Fixes: 8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing= their allocation order") Signed-off-by: Petr Pavlu Signed-off-by: Steven Rostedt (Google) --- kernel/trace/ring_buffer.c | 44 ++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index fb04445f92c3..3ea4f7bb1837 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -6728,39 +6728,38 @@ int ring_buffer_subbuf_order_set(struct trace_buffe= r *buffer, int order) } =20 for_each_buffer_cpu(buffer, cpu) { + struct buffer_data_page *old_free_data_page; + struct list_head old_pages; + unsigned long flags; =20 if (!cpumask_test_cpu(cpu, buffer->cpumask)) continue; =20 cpu_buffer =3D buffer->buffers[cpu]; =20 + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); + /* Clear the head bit to make the link list normal to read */ rb_head_page_deactivate(cpu_buffer); =20 - /* Now walk the list and free all the old sub buffers */ - list_for_each_entry_safe(bpage, tmp, cpu_buffer->pages, list) { - list_del_init(&bpage->list); - free_buffer_page(bpage); - } - /* The above loop stopped an the last page needing to be freed */ - bpage =3D list_entry(cpu_buffer->pages, struct buffer_page, list); - free_buffer_page(bpage); - - /* Free the current reader page */ - free_buffer_page(cpu_buffer->reader_page); + /* + * Collect buffers from the cpu_buffer pages list and the + * reader_page on old_pages, so they can be freed later when not + * under a spinlock. The pages list is a linked list with no + * head, adding old_pages turns it into a regular list with + * old_pages being the head. + */ + list_add(&old_pages, cpu_buffer->pages); + list_add(&cpu_buffer->reader_page->list, &old_pages); =20 /* One page was allocated for the reader page */ cpu_buffer->reader_page =3D list_entry(cpu_buffer->new_pages.next, struct buffer_page, list); list_del_init(&cpu_buffer->reader_page->list); =20 - /* The cpu_buffer pages are a link list with no head */ + /* Install the new pages, remove the head from the list */ cpu_buffer->pages =3D cpu_buffer->new_pages.next; - cpu_buffer->new_pages.next->prev =3D cpu_buffer->new_pages.prev; - cpu_buffer->new_pages.prev->next =3D cpu_buffer->new_pages.next; - - /* Clear the new_pages list */ - INIT_LIST_HEAD(&cpu_buffer->new_pages); + list_del_init(&cpu_buffer->new_pages); =20 cpu_buffer->head_page =3D list_entry(cpu_buffer->pages, struct buffer_page, list); @@ -6769,11 +6768,20 @@ int ring_buffer_subbuf_order_set(struct trace_buffe= r *buffer, int order) cpu_buffer->nr_pages =3D cpu_buffer->nr_pages_to_update; cpu_buffer->nr_pages_to_update =3D 0; =20 - free_pages((unsigned long)cpu_buffer->free_page, old_order); + old_free_data_page =3D cpu_buffer->free_page; cpu_buffer->free_page =3D NULL; =20 rb_head_page_activate(cpu_buffer); =20 + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); + + /* Free old sub buffers */ + list_for_each_entry_safe(bpage, tmp, &old_pages, list) { + list_del_init(&bpage->list); + free_buffer_page(bpage); + } + free_pages((unsigned long)old_free_data_page, old_order); + rb_check_pages(cpu_buffer); } =20 --=20 2.45.2