kcsan: Introduce report access_info and other_info

Improve readability by introducing access_info and other_info structs,
and in preparation of the following commit in this series replaces the
single instance of other_info with an array of size 1.

No functional change intended.

Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
This commit is contained in:
Marco Elver 2020-03-18 18:38:44 +01:00 committed by Paul E. McKenney
parent 1443b8c9e7
commit 135c0872d8
3 changed files with 77 additions and 78 deletions

View File

@ -321,7 +321,7 @@ static noinline void kcsan_found_watchpoint(const volatile void *ptr,
flags = user_access_save();
if (consumed) {
kcsan_report(ptr, size, type, true, raw_smp_processor_id(),
kcsan_report(ptr, size, type, KCSAN_VALUE_CHANGE_MAYBE,
KCSAN_REPORT_CONSUMED_WATCHPOINT);
} else {
/*
@ -500,8 +500,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
if (is_assert && value_change == KCSAN_VALUE_CHANGE_TRUE)
kcsan_counter_inc(KCSAN_COUNTER_ASSERT_FAILURES);
kcsan_report(ptr, size, type, value_change, raw_smp_processor_id(),
KCSAN_REPORT_RACE_SIGNAL);
kcsan_report(ptr, size, type, value_change, KCSAN_REPORT_RACE_SIGNAL);
} else if (value_change == KCSAN_VALUE_CHANGE_TRUE) {
/* Inferring a race, since the value should not have changed. */
@ -511,7 +510,6 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type)
if (IS_ENABLED(CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN) || is_assert)
kcsan_report(ptr, size, type, KCSAN_VALUE_CHANGE_TRUE,
raw_smp_processor_id(),
KCSAN_REPORT_RACE_UNKNOWN_ORIGIN);
}

View File

@ -135,7 +135,7 @@ enum kcsan_report_type {
* Print a race report from thread that encountered the race.
*/
extern void kcsan_report(const volatile void *ptr, size_t size, int access_type,
enum kcsan_value_change value_change, int cpu_id,
enum kcsan_value_change value_change,
enum kcsan_report_type type);
#endif /* _KERNEL_KCSAN_KCSAN_H */

View File

@ -19,18 +19,23 @@
*/
#define NUM_STACK_ENTRIES 64
/* Common access info. */
struct access_info {
const volatile void *ptr;
size_t size;
int access_type;
int task_pid;
int cpu_id;
};
/*
* Other thread info: communicated from other racing thread to thread that set
* up the watchpoint, which then prints the complete report atomically. Only
* need one struct, as all threads should to be serialized regardless to print
* the reports, with reporting being in the slow-path.
*/
static struct {
const volatile void *ptr;
size_t size;
int access_type;
int task_pid;
int cpu_id;
struct other_info {
struct access_info ai;
unsigned long stack_entries[NUM_STACK_ENTRIES];
int num_stack_entries;
@ -52,7 +57,9 @@ static struct {
* that populated @other_info until it has been consumed.
*/
struct task_struct *task;
} other_info;
};
static struct other_info other_infos[1];
/*
* Information about reported races; used to rate limit reporting.
@ -238,7 +245,7 @@ static const char *get_thread_desc(int task_id)
}
/* Helper to skip KCSAN-related functions in stack-trace. */
static int get_stack_skipnr(unsigned long stack_entries[], int num_entries)
static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries)
{
char buf[64];
int skip = 0;
@ -279,9 +286,10 @@ static void print_verbose_info(struct task_struct *task)
/*
* Returns true if a report was generated, false otherwise.
*/
static bool print_report(const volatile void *ptr, size_t size, int access_type,
enum kcsan_value_change value_change, int cpu_id,
enum kcsan_report_type type)
static bool print_report(enum kcsan_value_change value_change,
enum kcsan_report_type type,
const struct access_info *ai,
const struct other_info *other_info)
{
unsigned long stack_entries[NUM_STACK_ENTRIES] = { 0 };
int num_stack_entries = stack_trace_save(stack_entries, NUM_STACK_ENTRIES, 1);
@ -297,9 +305,9 @@ static bool print_report(const volatile void *ptr, size_t size, int access_type,
return false;
if (type == KCSAN_REPORT_RACE_SIGNAL) {
other_skipnr = get_stack_skipnr(other_info.stack_entries,
other_info.num_stack_entries);
other_frame = other_info.stack_entries[other_skipnr];
other_skipnr = get_stack_skipnr(other_info->stack_entries,
other_info->num_stack_entries);
other_frame = other_info->stack_entries[other_skipnr];
/* @value_change is only known for the other thread */
if (skip_report(value_change, other_frame))
@ -321,13 +329,13 @@ static bool print_report(const volatile void *ptr, size_t size, int access_type,
*/
cmp = sym_strcmp((void *)other_frame, (void *)this_frame);
pr_err("BUG: KCSAN: %s in %ps / %ps\n",
get_bug_type(access_type | other_info.access_type),
get_bug_type(ai->access_type | other_info->ai.access_type),
(void *)(cmp < 0 ? other_frame : this_frame),
(void *)(cmp < 0 ? this_frame : other_frame));
} break;
case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
pr_err("BUG: KCSAN: %s in %pS\n", get_bug_type(access_type),
pr_err("BUG: KCSAN: %s in %pS\n", get_bug_type(ai->access_type),
(void *)this_frame);
break;
@ -341,30 +349,28 @@ static bool print_report(const volatile void *ptr, size_t size, int access_type,
switch (type) {
case KCSAN_REPORT_RACE_SIGNAL:
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(other_info.access_type), other_info.ptr,
other_info.size, get_thread_desc(other_info.task_pid),
other_info.cpu_id);
get_access_type(other_info->ai.access_type), other_info->ai.ptr,
other_info->ai.size, get_thread_desc(other_info->ai.task_pid),
other_info->ai.cpu_id);
/* Print the other thread's stack trace. */
stack_trace_print(other_info.stack_entries + other_skipnr,
other_info.num_stack_entries - other_skipnr,
stack_trace_print(other_info->stack_entries + other_skipnr,
other_info->num_stack_entries - other_skipnr,
0);
if (IS_ENABLED(CONFIG_KCSAN_VERBOSE))
print_verbose_info(other_info.task);
print_verbose_info(other_info->task);
pr_err("\n");
pr_err("%s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(access_type), ptr, size,
get_thread_desc(in_task() ? task_pid_nr(current) : -1),
cpu_id);
get_access_type(ai->access_type), ai->ptr, ai->size,
get_thread_desc(ai->task_pid), ai->cpu_id);
break;
case KCSAN_REPORT_RACE_UNKNOWN_ORIGIN:
pr_err("race at unknown origin, with %s to 0x%px of %zu bytes by %s on cpu %i:\n",
get_access_type(access_type), ptr, size,
get_thread_desc(in_task() ? task_pid_nr(current) : -1),
cpu_id);
get_access_type(ai->access_type), ai->ptr, ai->size,
get_thread_desc(ai->task_pid), ai->cpu_id);
break;
default:
@ -386,22 +392,23 @@ static bool print_report(const volatile void *ptr, size_t size, int access_type,
return true;
}
static void release_report(unsigned long *flags, enum kcsan_report_type type)
static void release_report(unsigned long *flags, struct other_info *other_info)
{
if (type == KCSAN_REPORT_RACE_SIGNAL)
other_info.ptr = NULL; /* mark for reuse */
if (other_info)
other_info->ai.ptr = NULL; /* Mark for reuse. */
spin_unlock_irqrestore(&report_lock, *flags);
}
/*
* Sets @other_info.task and awaits consumption of @other_info.
* Sets @other_info->task and awaits consumption of @other_info.
*
* Precondition: report_lock is held.
* Postcondition: report_lock is held.
*/
static void
set_other_info_task_blocking(unsigned long *flags, const volatile void *ptr)
static void set_other_info_task_blocking(unsigned long *flags,
const struct access_info *ai,
struct other_info *other_info)
{
/*
* We may be instrumenting a code-path where current->state is already
@ -418,7 +425,7 @@ set_other_info_task_blocking(unsigned long *flags, const volatile void *ptr)
*/
int timeout = max(kcsan_udelay_task, kcsan_udelay_interrupt);
other_info.task = current;
other_info->task = current;
do {
if (is_running) {
/*
@ -438,19 +445,19 @@ set_other_info_task_blocking(unsigned long *flags, const volatile void *ptr)
spin_lock_irqsave(&report_lock, *flags);
if (timeout-- < 0) {
/*
* Abort. Reset other_info.task to NULL, since it
* Abort. Reset @other_info->task to NULL, since it
* appears the other thread is still going to consume
* it. It will result in no verbose info printed for
* this task.
*/
other_info.task = NULL;
other_info->task = NULL;
break;
}
/*
* If @ptr nor @current matches, then our information has been
* consumed and we may continue. If not, retry.
*/
} while (other_info.ptr == ptr && other_info.task == current);
} while (other_info->ai.ptr == ai->ptr && other_info->task == current);
if (is_running)
set_current_state(TASK_RUNNING);
}
@ -460,9 +467,8 @@ set_other_info_task_blocking(unsigned long *flags, const volatile void *ptr)
* acquires the matching other_info and returns true. If other_info is not
* required for the report type, simply acquires report_lock and returns true.
*/
static bool prepare_report(unsigned long *flags, const volatile void *ptr,
size_t size, int access_type, int cpu_id,
enum kcsan_report_type type)
static bool prepare_report(unsigned long *flags, enum kcsan_report_type type,
const struct access_info *ai, struct other_info *other_info)
{
if (type != KCSAN_REPORT_CONSUMED_WATCHPOINT &&
type != KCSAN_REPORT_RACE_SIGNAL) {
@ -476,18 +482,14 @@ retry:
switch (type) {
case KCSAN_REPORT_CONSUMED_WATCHPOINT:
if (other_info.ptr != NULL)
if (other_info->ai.ptr)
break; /* still in use, retry */
other_info.ptr = ptr;
other_info.size = size;
other_info.access_type = access_type;
other_info.task_pid = in_task() ? task_pid_nr(current) : -1;
other_info.cpu_id = cpu_id;
other_info.num_stack_entries = stack_trace_save(other_info.stack_entries, NUM_STACK_ENTRIES, 1);
other_info->ai = *ai;
other_info->num_stack_entries = stack_trace_save(other_info->stack_entries, NUM_STACK_ENTRIES, 1);
if (IS_ENABLED(CONFIG_KCSAN_VERBOSE))
set_other_info_task_blocking(flags, ptr);
set_other_info_task_blocking(flags, ai, other_info);
spin_unlock_irqrestore(&report_lock, *flags);
@ -498,37 +500,31 @@ retry:
return false;
case KCSAN_REPORT_RACE_SIGNAL:
if (other_info.ptr == NULL)
if (!other_info->ai.ptr)
break; /* no data available yet, retry */
/*
* First check if this is the other_info we are expecting, i.e.
* matches based on how watchpoint was encoded.
*/
if (!matching_access((unsigned long)other_info.ptr &
WATCHPOINT_ADDR_MASK,
other_info.size,
(unsigned long)ptr & WATCHPOINT_ADDR_MASK,
size))
if (!matching_access((unsigned long)other_info->ai.ptr & WATCHPOINT_ADDR_MASK, other_info->ai.size,
(unsigned long)ai->ptr & WATCHPOINT_ADDR_MASK, ai->size))
break; /* mismatching watchpoint, retry */
if (!matching_access((unsigned long)other_info.ptr,
other_info.size, (unsigned long)ptr,
size)) {
if (!matching_access((unsigned long)other_info->ai.ptr, other_info->ai.size,
(unsigned long)ai->ptr, ai->size)) {
/*
* If the actual accesses to not match, this was a false
* positive due to watchpoint encoding.
*/
kcsan_counter_inc(
KCSAN_COUNTER_ENCODING_FALSE_POSITIVES);
kcsan_counter_inc(KCSAN_COUNTER_ENCODING_FALSE_POSITIVES);
/* discard this other_info */
release_report(flags, KCSAN_REPORT_RACE_SIGNAL);
release_report(flags, other_info);
return false;
}
access_type |= other_info.access_type;
if ((access_type & KCSAN_ACCESS_WRITE) == 0) {
if (!((ai->access_type | other_info->ai.access_type) & KCSAN_ACCESS_WRITE)) {
/*
* While the address matches, this is not the other_info
* from the thread that consumed our watchpoint, since
@ -561,15 +557,11 @@ retry:
* data, and at this point the likelihood that we
* re-report the same race again is high.
*/
release_report(flags, KCSAN_REPORT_RACE_SIGNAL);
release_report(flags, other_info);
return false;
}
/*
* Matching & usable access in other_info: keep other_info_lock
* locked, as this thread consumes it to print the full report;
* unlocked in release_report.
*/
/* Matching access in other_info. */
return true;
default:
@ -582,10 +574,19 @@ retry:
}
void kcsan_report(const volatile void *ptr, size_t size, int access_type,
enum kcsan_value_change value_change, int cpu_id,
enum kcsan_value_change value_change,
enum kcsan_report_type type)
{
unsigned long flags = 0;
const struct access_info ai = {
.ptr = ptr,
.size = size,
.access_type = access_type,
.task_pid = in_task() ? task_pid_nr(current) : -1,
.cpu_id = raw_smp_processor_id()
};
struct other_info *other_info = type == KCSAN_REPORT_RACE_UNKNOWN_ORIGIN
? NULL : &other_infos[0];
/*
* With TRACE_IRQFLAGS, lockdep's IRQ trace state becomes corrupted if
@ -596,19 +597,19 @@ void kcsan_report(const volatile void *ptr, size_t size, int access_type,
lockdep_off();
kcsan_disable_current();
if (prepare_report(&flags, ptr, size, access_type, cpu_id, type)) {
if (prepare_report(&flags, type, &ai, other_info)) {
/*
* Never report if value_change is FALSE, only if we it is
* either TRUE or MAYBE. In case of MAYBE, further filtering may
* be done once we know the full stack trace in print_report().
*/
bool reported = value_change != KCSAN_VALUE_CHANGE_FALSE &&
print_report(ptr, size, access_type, value_change, cpu_id, type);
print_report(value_change, type, &ai, other_info);
if (reported && panic_on_warn)
panic("panic_on_warn set ...\n");
release_report(&flags, type);
release_report(&flags, other_info);
}
kcsan_enable_current();