[v2] kbuffer: Always walk the events to calculate timestamp in kbuffer_read_buffer()

Commit Message

Steven Rostedt Jan. 8, 2024, 5:34 p.m. UTC

From: "Steven Rostedt (Google)" <[e-mail]>

If the buffer to read the kbuffer subbuffer is big enough to hold the
entire buffer, it was simply copied. But this is not good enough, as the
next read should include the events after what was copied. That means the
timestamps need to be calculated.

Just copying the data is not enough as the timestamp of any event is an
offset of the previous event. To know the event timestamp after what was
copied, the timestamps of the events before it need to be read, to know
what the offset is against of the following event.

Link: https://lore.kernel.org/linux-trace-devel/[e-mail]/

Fixes: 05821189 ("kbuffer: Add kbuffer_read_buffer()")
Reported-by: Vincent Donnefort <[e-mail]>
Signed-off-by: Steven Rostedt (Google) <[e-mail]>
---
Changes from v1: https://lore.kernel.org/linux-trace-devel/[e-mail]
 (which was originally: kbuffer: Add event if the buffer just fits in kbuffer_read_buffer()

 - Do not just fix the copying by the off-by-one error, but instead always
   read all events to copy to calculate the timestamp (Vincent Donnefort)

 src/kbuffer-parse.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

Patch

diff --git a/src/kbuffer-parse.c b/src/kbuffer-parse.c
index d43fe5d972fd..691d53678f5f 100644
--- a/src/kbuffer-parse.c
+++ b/src/kbuffer-parse.c
@@ -947,19 +947,12 @@  kbuffer_raw_get(struct kbuffer *kbuf, void *subbuf, struct kbuffer_raw_info *inf
  */
 int kbuffer_read_buffer(struct kbuffer *kbuf, void *buffer, int len)
 {
-	int subbuf_size = kbuf->start + kbuf->size;
 	unsigned long long ts;
 	unsigned int type_len_ts;
 	bool do_swap = false;
 	int last_next;
 	int save_curr;
 
-	if (!kbuf->curr && len >= subbuf_size) {
-		memcpy(buffer, kbuf->subbuffer, subbuf_size);
-		set_curr_to_end(kbuf);
-		return kbuf->size;
-	}
-
 	/* Are we at the end of the buffer */
 	if (kbuf->curr >= kbuf->size)
 		return 0;
@@ -982,24 +975,13 @@  int kbuffer_read_buffer(struct kbuffer *kbuf, void *buffer, int len)
 
 	save_curr = kbuf->curr;
 
-	/* Copy the rest of the buffer if it fits */
-	if (len >= kbuf->size - kbuf->curr) {
-		set_curr_to_end(kbuf);
-		last_next = kbuf->size;
-	} else {
-		/*
-		 * The length doesn't hold the rest,
-		 * need to find the last that fits
-		 */
+	/* Due to timestamps, we must save the current next to use */
+	last_next = kbuf->next;
 
-		/* Due to timestamps, we must save the current next to use */
+	while (len >= kbuf->next - save_curr) {
 		last_next = kbuf->next;
-
-		while (len > kbuf->next - save_curr) {
-			last_next = kbuf->next;
-			if (!kbuffer_next_event(kbuf, &ts))
-				break;
-		}
+		if (!kbuffer_next_event(kbuf, &ts))
+			break;
 	}
 
 	len = last_next - save_curr;