Skip to content

Commit

Permalink
ring-buffer: Fix a race in rb_time_cmpxchg() for 32 bit archs
Browse files Browse the repository at this point in the history
Mathieu Desnoyers pointed out an issue in the rb_time_cmpxchg() for 32 bit
architectures. That is:

 static bool rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set)
 {
	unsigned long cnt, top, bottom, msb;
	unsigned long cnt2, top2, bottom2, msb2;
	u64 val;

	/* The cmpxchg always fails if it interrupted an update */
	 if (!__rb_time_read(t, &val, &cnt2))
		 return false;

	 if (val != expect)
		 return false;

<<<< interrupted here!

	 cnt = local_read(&t->cnt);

The problem is that the synchronization counter in the rb_time_t is read
*after* the value of the timestamp is read. That means if an interrupt
were to come in between the value being read and the counter being read,
it can change the value and the counter and the interrupted process would
be clueless about it!

The counter needs to be read first and then the value. That way it is easy
to tell if the value is stale or not. If the counter hasn't been updated,
then the value is still good.

Link: https://lore.kernel.org/linux-trace-kernel/[email protected]/
Link: https://lore.kernel.org/linux-trace-kernel/[email protected]

Cc: [email protected]
Cc: Masami Hiramatsu <[email protected]>
Cc: Mark Rutland <[email protected]>
Fixes: 10464b4 ("ring-buffer: Add rb_time_t 64 bit operations for speeding up 32 bit")
Reported-by: Mathieu Desnoyers <[email protected]>
Reviewed-by: Mathieu Desnoyers <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
  • Loading branch information
rostedt committed Dec 15, 2023
1 parent 083e9f6 commit fff88fa
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion kernel/trace/ring_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -706,14 +706,16 @@ static bool rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set)
unsigned long cnt2, top2, bottom2, msb2;
u64 val;

/* Any interruptions in this function should cause a failure */
cnt = local_read(&t->cnt);

/* The cmpxchg always fails if it interrupted an update */
if (!__rb_time_read(t, &val, &cnt2))
return false;

if (val != expect)
return false;

cnt = local_read(&t->cnt);
if ((cnt & 3) != cnt2)
return false;

Expand Down

0 comments on commit fff88fa

Please sign in to comment.