The ftrace ring buffer always uses the v2 (blk_io_trace2) format, but
__blk_add_trace() switched the reserve size and record format on
bt->version. That field only describes the relay/classic blktrace record
format and must not change what goes into the ftrace buffer: the ftrace
readers (print_one_line() and friends) unconditionally parse blk_io_trace2.
The BLKTRACESETUP ioctl sets bt->version to 1. With the blk tracer also
enabled, __blk_add_trace() recorded a 48-byte v1 event into the ftrace
ring buffer, but the reader parses the 64-byte v2 layout, so pdu_start()
points 16 bytes past the PDU and blk_log_remap() reads out of bounds - a
use-after-free when the ring buffer page is resized concurrently.
Always use the v2 format in the blk_tracer path, and initialize
bt->version to 2 in blk_trace_setup_queue() so the sysfs-enabled path no
longer leaves it uninitialized.
Fixes: e48886b9d668 ("blktrace: for ftrace use correct trace format ver")
Reported-by: [email protected]
Link: https://syzkaller.appspot.com/bug?extid=4dfd96209d744263a972
Tested-by: [email protected]
Cc: [email protected]
Signed-off-by: Adriano Cordova <[email protected]>
---
kernel/trace/blktrace.c | 74 +++++++++++------------------------------
1 file changed, 20 insertions(+), 54 deletions(-)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 8cd2520b4c99..ae010969c144 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -385,66 +385,26 @@ static void __blk_add_trace(struct blk_trace *bt,
sector_t sector, int bytes,
if (blk_tracer) {
buffer = blk_tr->array_buffer.buffer;
trace_ctx = tracing_gen_ctx_flags(0);
- switch (bt->version) {
- case 1:
- trace_len = sizeof(struct blk_io_trace);
- break;
- case 2:
- default:
- /*
- * ftrace always uses v2 (blk_io_trace2) format.
- *
- * For sysfs-enabled tracing path (enabled via
- * /sys/block/DEV/trace/enable), blk_trace_setup_queue()
- * never initializes bt->version, leaving it 0 from
- * kzalloc(). We must handle version==0 safely here.
- *
- * Fall through to default to ensure we never hit the
- * old bug where default set trace_len=0, causing
- * buffer underflow and memory corruption.
- *
- * Always use v2 format for ftrace and normalize
- * bt->version to 2 when uninitialized.
- */
- trace_len = sizeof(struct blk_io_trace2);
- if (bt->version == 0)
- bt->version = 2;
- break;
- }
- trace_len += pdu_len + cgid_len;
+ /*
+ * The ftrace ring buffer always uses the v2 (blk_io_trace2)
+ * format; the ftrace readers parse only that. bt->version
+ * describes just the relay/classic blktrace record format.
+ * Recording a v1-sized event here would shift pdu_start() 16
+ * bytes past the PDU and make blk_log_remap() read past the
+ * event (a use-after-free on concurrent buffer resize), and v1
+ * cannot represent the newer 64-bit zone actions anyway.
+ */
+ trace_len = sizeof(struct blk_io_trace2) + pdu_len + cgid_len;
event = trace_buffer_lock_reserve(buffer, TRACE_BLK,
trace_len, trace_ctx);
if (!event)
return;
tracing_record_cmdline(current);
- switch (bt->version) {
- case 1:
- record_blktrace_event(ring_buffer_event_data(event),
- pid, cpu, sector, bytes,
- what, bt->dev, error, cgid,
cgid_len,
- pdu_data, pdu_len);
- break;
- case 2:
- default:
- /*
- * Use v2 recording function (record_blktrace_event2)
- * which writes blk_io_trace2 structure with correct
- * field layout:
- * - 32-bit pid at offset 28
- * - 64-bit action at offset 32
- *
- * Fall through to default handles version==0 case
- * (from sysfs path), ensuring we always use correct
- * v2 recording function to match the v2 buffer
- * allocated above.
- */
- record_blktrace_event2(ring_buffer_event_data(event),
- pid, cpu, sector, bytes,
- what, bt->dev, error, cgid,
cgid_len,
- pdu_data, pdu_len);
- break;
- }
+ record_blktrace_event2(ring_buffer_event_data(event),
+ pid, cpu, sector, bytes,
+ what, bt->dev, error, cgid, cgid_len,
+ pdu_data, pdu_len);
trace_buffer_unlock_commit(blk_tr, buffer, event, trace_ctx);
return;
@@ -1913,6 +1873,12 @@ static int blk_trace_setup_queue(struct request_queue *q,
bt->dev = bdev->bd_dev;
bt->act_mask = (u16)-1;
+ /*
+ * This sysfs-enabled path feeds the ftrace blk tracer, which always
+ * uses the v2 (blk_io_trace2) format. Initialize the version so it is
+ * never left dangling as 0 for future consumers.
+ */
+ bt->version = 2;
blk_trace_setup_lba(bt, bdev);
--
2.51.0
