From: Steven Rostedt <rostedt@goodmis.org>
To: contact me <www@aegistudio.net>
Cc: "Linus Torvalds" <torvalds@linuxfoundation.org>,
"Ingo Molnar" <mingo@redhat.com>,
"linux-trace-devel" <linux-trace-devel@vger.kernel.org>,
"Security Officers" <security@kernel.org>
Subject: Re: [PATCH 1/1] tracing: fix bug in rb_per_cpu_empty() that might cause deadloop.
Date: Tue, 20 Jul 2021 22:48:56 -0400 [thread overview]
Message-ID: <20210720224856.7a19cf40@oasis.local.home> (raw)
In-Reply-To: <17ac6d675f2.129c41f20168460.6720846023977971963@aegistudio.net>
On Wed, 21 Jul 2021 10:13:01 +0800
contact me <www@aegistudio.net> wrote:
> I'll have a try and tell you the result. But I've seen the code written on
Thanks.
>
> https://github.com/torvalds/linux/blob/2734d6c1b1a089fb593ef6a23d4b70903526fe0c/kernel/trace/ring_buffer.c#L4513
> cpu_buffer->reader_page->read = 0;
Yeah, that clears the "read" for the reader page after it gets swapped
out of the writers buffer, and before the reader gets to it.
>
> I though the algorithm is to reset the reader_page->read when it swaps with the
> head page. And following the original algorithm the read field of a buffer page
> should be either 0 or its original value when the content to read exhaust. So I
> added an extra conditional to ensure that.
Actually, we could just change it to ignore read as well, and that
could be the fix.
Basically, the bug is that we use that "read" field that's in the
writer's buffer (the main ring buffer outside the reader page), and it
has stale data.
So we either need to clear the "read" before placing it back into the
writer's buffer (which my patch does). Or we can simply ignore it,
which would change your patch to:
return reader->read == rb_page_commit(reader) &&
(commit == reader ||
(commit == head &&
- head->read == rb_page_commit(commit)));
+ (rb_page_commit(commit) == 0)));
}
In which case, you can send me that patch with the cleanup that Linus
suggested, and my commenting.
-- Steve
prev parent reply other threads:[~2021-07-21 2:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-20 8:48 [PATCH 1/1] tracing: fix bug in rb_per_cpu_empty() that might cause deadloop Haoran Luo
2021-07-20 18:35 ` Linus Torvalds
2021-07-20 22:36 ` contact me
2021-07-21 1:10 ` Steven Rostedt
2021-07-21 1:08 ` Steven Rostedt
2021-07-21 2:13 ` contact me
2021-07-21 2:48 ` Steven Rostedt [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210720224856.7a19cf40@oasis.local.home \
--to=rostedt@goodmis.org \
--cc=linux-trace-devel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=security@kernel.org \
--cc=torvalds@linuxfoundation.org \
--cc=www@aegistudio.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).