Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] tracing: shared-memory TOCTOU in simple_ring_buffer_reserve()
--
commit 9257cd3bebb215fd4b1122136bcc3bcfeab103ba
Author: Vincent Donnefort [off-list ref]
tracing/simple_ring_buffer: Add support for compressed length
This commit compresses small event lengths into the ring buffer event header
type_len field and enforces ring-buffer alignment to save 4 bytes per small
event.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/simple_ring_buffer.c b/kernel/trace/simple_ring_buffer.c
index abfaf9bee12e1..a305331d29ba9 100644
--- a/kernel/trace/simple_ring_buffer.c
+++ b/kernel/trace/simple_ring_buffer.c
@@ -207,7 +207,13 @@ static unsigned long rb_event_size(unsigned long length)
{
struct ring_buffer_event *event;
- return length + RB_EVNT_HDR_SIZE + sizeof(event->array[0]);
+ if (!length)
+ length++;
+
+ if (length > RB_MAX_SMALL_DATA || RB_FORCE_8BYTE_ALIGNMENT)
+ length += sizeof(event->array[0]);
+
+ return ALIGN(length + RB_EVNT_HDR_SIZE, RB_ARCH_ALIGNMENT);
}
static struct ring_buffer_event *@@ -223,12 +229,15 @@ rb_event_add_ts_extend(struct ring_buffer_event *event, u64 delta)
static struct ring_buffer_event *
simple_rb_reserve_next(struct simple_rb_per_cpu *cpu_buffer, unsigned long length, u64 timestamp)
{
- unsigned long ts_ext_size = 0, event_size = rb_event_size(length);
struct simple_buffer_page *tail = cpu_buffer->tail_page;
+ unsigned long event_size, array_size, ts_ext_size = 0;
struct ring_buffer_event *event;
u32 write, prev_write;
u64 time_delta;
+ event_size = rb_event_size(length);
+ array_size = event_size - RB_EVNT_HDR_SIZE;
+
time_delta = timestamp - cpu_buffer->write_stamp;
if (test_time_stamp(time_delta))@@ -259,9 +268,13 @@ simple_rb_reserve_next(struct simple_rb_per_cpu *cpu_buffer, unsigned long lengt
time_delta = 0;
}
- event->type_len = 0;
+ if (length > RB_MAX_SMALL_DATA || RB_FORCE_8BYTE_ALIGNMENT) {
+ event->type_len = 0;
+ event->array[0] = array_size;
+ } else {
+ event->type_len = DIV_ROUND_UP(array_size, RB_ALIGNMENT);
+ }
event->time_delta = time_delta;
- event->array[0] = event_size - RB_EVNT_HDR_SIZE;
return event;
}@@ -284,7 +297,7 @@ void *simple_ring_buffer_reserve(struct simple_rb_per_cpu *cpu_buffer, unsigned
rb_event = simple_rb_reserve_next(cpu_buffer, length, timestamp);
- return &rb_event->array[1];
+ return rb_event->type_len ? &rb_event->array[0] : &rb_event->array[1];
[Severity: High]
Can reading rb_event->type_len back from the shared buffer page cause a
4-byte out-of-bounds write at EL2?
When compiled into the arm64 pKVM hypervisor (arch/arm64/kvm/hyp/nvhe/trace.c),
tail->page is shared with the untrusted host kernel via hyp_pin_shared_mem(),
and RB_FORCE_8BYTE_ALIGNMENT is 0. For length <= RB_MAX_SMALL_DATA,
rb_event_size() does not reserve space for event->array[0], but
simple_ring_buffer_reserve() re-reads rb_event->type_len from shared memory
to choose between &rb_event->array[0] and &rb_event->array[1]:
CPU0 (EL2 hypervisor) CPU1 (EL1 host)
simple_ring_buffer_reserve()
simple_rb_reserve_next()
event->type_len = ...
event->time_delta = ...
rb_event->type_len = 0;
if (rb_event->type_len)
...
return &rb_event->array[1]; <-- 4 bytes past reserved space
If an event is reserved at the end of a page (write == PAGE_SIZE -
BUF_PAGE_HDR_SIZE), the caller writes 4 bytes past the end of tail->page in
EL2 memory. Checking (length > RB_MAX_SMALL_DATA || RB_FORCE_8BYTE_ALIGNMENT)
instead of rb_event->type_len avoids reading back from shared memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106180657.2460376-1-vdonnefort@google.com?part=7