Skip to content

Commit be3f696

Browse files
melverpaulmckrcu
authored andcommitted
kcsan: Show location access was reordered to
Also show the location the access was reordered to. An example report: | ================================================================== | BUG: KCSAN: data-race in test_kernel_wrong_memorder / test_kernel_wrong_memorder | | read-write to 0xffffffffc01e61a8 of 8 bytes by task 2311 on cpu 5: | test_kernel_wrong_memorder+0x57/0x90 | access_thread+0x99/0xe0 | kthread+0x2ba/0x2f0 | ret_from_fork+0x22/0x30 | | read-write (reordered) to 0xffffffffc01e61a8 of 8 bytes by task 2310 on cpu 7: | test_kernel_wrong_memorder+0x57/0x90 | access_thread+0x99/0xe0 | kthread+0x2ba/0x2f0 | ret_from_fork+0x22/0x30 | | | +-> reordered to: test_kernel_wrong_memorder+0x80/0x90 | | Reported by Kernel Concurrency Sanitizer on: | CPU: 7 PID: 2310 Comm: access_thread Not tainted 5.14.0-rc1+ #18 | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014 | ================================================================== Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
1 parent 3cc21a5 commit be3f696

File tree

1 file changed

+23
-12
lines changed

1 file changed

+23
-12
lines changed

kernel/kcsan/report.c

+23-12
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,12 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries
308308

309309
/*
310310
* Skips to the first entry that matches the function of @ip, and then replaces
311-
* that entry with @ip, returning the entries to skip.
311+
* that entry with @ip, returning the entries to skip with @replaced containing
312+
* the replaced entry.
312313
*/
313314
static int
314-
replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned long ip)
315+
replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned long ip,
316+
unsigned long *replaced)
315317
{
316318
unsigned long symbolsize, offset;
317319
unsigned long target_func;
@@ -330,6 +332,7 @@ replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned lon
330332
func -= offset;
331333

332334
if (func == target_func) {
335+
*replaced = stack_entries[skip];
333336
stack_entries[skip] = ip;
334337
return skip;
335338
}
@@ -342,9 +345,10 @@ replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned lon
342345
}
343346

344347
static int
345-
sanitize_stack_entries(unsigned long stack_entries[], int num_entries, unsigned long ip)
348+
sanitize_stack_entries(unsigned long stack_entries[], int num_entries, unsigned long ip,
349+
unsigned long *replaced)
346350
{
347-
return ip ? replace_stack_entry(stack_entries, num_entries, ip) :
351+
return ip ? replace_stack_entry(stack_entries, num_entries, ip, replaced) :
348352
get_stack_skipnr(stack_entries, num_entries);
349353
}
350354

@@ -360,6 +364,14 @@ static int sym_strcmp(void *addr1, void *addr2)
360364
return strncmp(buf1, buf2, sizeof(buf1));
361365
}
362366

367+
static void
368+
print_stack_trace(unsigned long stack_entries[], int num_entries, unsigned long reordered_to)
369+
{
370+
stack_trace_print(stack_entries, num_entries, 0);
371+
if (reordered_to)
372+
pr_err(" |\n +-> reordered to: %pS\n", (void *)reordered_to);
373+
}
374+
363375
static void print_verbose_info(struct task_struct *task)
364376
{
365377
if (!task)
@@ -378,10 +390,12 @@ static void print_report(enum kcsan_value_change value_change,
378390
struct other_info *other_info,
379391
u64 old, u64 new, u64 mask)
380392
{
393+
unsigned long reordered_to = 0;
381394
unsigned long stack_entries[NUM_STACK_ENTRIES] = { 0 };
382395
int num_stack_entries = stack_trace_save(stack_entries, NUM_STACK_ENTRIES, 1);
383-
int skipnr = sanitize_stack_entries(stack_entries, num_stack_entries, ai->ip);
396+
int skipnr = sanitize_stack_entries(stack_entries, num_stack_entries, ai->ip, &reordered_to);
384397
unsigned long this_frame = stack_entries[skipnr];
398+
unsigned long other_reordered_to = 0;
385399
unsigned long other_frame = 0;
386400
int other_skipnr = 0; /* silence uninit warnings */
387401

@@ -394,7 +408,7 @@ static void print_report(enum kcsan_value_change value_change,
394408
if (other_info) {
395409
other_skipnr = sanitize_stack_entries(other_info->stack_entries,
396410
other_info->num_stack_entries,
397-
other_info->ai.ip);
411+
other_info->ai.ip, &other_reordered_to);
398412
other_frame = other_info->stack_entries[other_skipnr];
399413

400414
/* @value_change is only known for the other thread */
@@ -434,10 +448,9 @@ static void print_report(enum kcsan_value_change value_change,
434448
other_info->ai.cpu_id);
435449

436450
/* Print the other thread's stack trace. */
437-
stack_trace_print(other_info->stack_entries + other_skipnr,
451+
print_stack_trace(other_info->stack_entries + other_skipnr,
438452
other_info->num_stack_entries - other_skipnr,
439-
0);
440-
453+
other_reordered_to);
441454
if (IS_ENABLED(CONFIG_KCSAN_VERBOSE))
442455
print_verbose_info(other_info->task);
443456

@@ -451,9 +464,7 @@ static void print_report(enum kcsan_value_change value_change,
451464
get_thread_desc(ai->task_pid), ai->cpu_id);
452465
}
453466
/* Print stack trace of this thread. */
454-
stack_trace_print(stack_entries + skipnr, num_stack_entries - skipnr,
455-
0);
456-
467+
print_stack_trace(stack_entries + skipnr, num_stack_entries - skipnr, reordered_to);
457468
if (IS_ENABLED(CONFIG_KCSAN_VERBOSE))
458469
print_verbose_info(current);
459470

0 commit comments

Comments
 (0)