All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sharp <dhsharp@google.com>
To: rostedt@goodmis.org
Cc: linux-kernel@vger.kernel.org, mrubin@google.com,
	David Sharp <dhsharp@google.com>
Subject: [PATCH] ring_buffer: off-by-one and duplicate events in ring_buffer_read_page
Date: Wed, 22 Dec 2010 16:38:24 -0800	[thread overview]
Message-ID: <1293064704-8101-1-git-send-email-dhsharp@google.com> (raw)
In-Reply-To: <AANLkTin7nLrRPc9qGjdjHbeVDDWiJjAiYyb-L=gH85bx@mail.gmail.com>

Fix two related problems in the event-copying loop of
ring_buffer_read_page.

The loop condition for copying events is off-by-one.
"len" is the remaining space in the caller-supplied page.
"size" is the size of the next event (or two events).
If len == size, then there is just enough space for the next event.

size was set to rb_event_ts_length, which may include the size of two
events if the first event is a time-extend, in order to assure time-
extends are kept together with the event after it. However,
rb_advance_reader always advances by one event. This would result in the
event after any time-extend being duplicated. Instead, get the size of
a single event for the memcpy, but use rb_event_ts_length for the loop
condition.

Signed-off-by: David Sharp <dhsharp@google.com>
CC: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <AANLkTin7nLrRPc9qGjdjHbeVDDWiJjAiYyb-L=gH85bx@mail.gmail.com>
---
 kernel/trace/ring_buffer.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 9ed509a..bd1c35a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3853,6 +3853,13 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
 
 		/* Need to copy one event at a time */
 		do {
+			/* We need the size of one event, because
+			 * rb_advance_reader only advances by one event,
+			 * whereas rb_event_ts_length may include the size of
+			 * one or two events.
+			 * We have already ensured there's enough space if this
+			 * is a time extend. */
+			size = rb_event_length(event);
 			memcpy(bpage->data + pos, rpage->data + rpos, size);
 
 			len -= size;
@@ -3867,7 +3874,7 @@ int ring_buffer_read_page(struct ring_buffer *buffer,
 			event = rb_reader_event(cpu_buffer);
 			/* Always keep the time extend and data together */
 			size = rb_event_ts_length(event);
-		} while (len > size);
+		} while (len >= size);
 
 		/* update bpage */
 		local_set(&bpage->commit, pos);
-- 
1.7.3.1


  parent reply	other threads:[~2010-12-23  0:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-21  2:43 ftrace: trace_pipe_raw interface broken David Sharp
2010-12-21  3:52 ` Steven Rostedt
2010-12-21  5:59   ` David Sharp
2010-12-22 23:23     ` David Sharp
2010-12-22 23:47       ` David Sharp
2010-12-23  0:37         ` David Sharp
2010-12-23 15:57           ` Steven Rostedt
2010-12-23 23:06             ` David Sharp
2010-12-24  4:35               ` Steven Rostedt
2010-12-22 23:45     ` [PATCH] ring_buffer: off-by-one and duplicate events in ring_buffer_read_page David Sharp
2010-12-23  0:38     ` David Sharp [this message]
2010-12-25  8:59     ` [tip:perf/urgent] ring_buffer: Off-by-one " tip-bot for David Sharp

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=1293064704-8101-1-git-send-email-dhsharp@google.com \
    --to=dhsharp@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mrubin@google.com \
    --cc=rostedt@goodmis.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.