Skip to content

Commit

Permalink
kcsan: Make barrier tests compatible with lockdep
Browse files Browse the repository at this point in the history 
The barrier tests in selftest and the kcsan_test module only need the
spinlock and mutex to test correct barrier instrumentation. Therefore,
these were initially placed on the stack.

However, lockdep asserts that locks are in static storage, and will
generate this warning:

 | INFO: trying to register non-static key.
 | The code is fine but needs lockdep annotation, or maybe
 | you didn't initialize this object before use?
 | turning off the locking correctness validator.
 | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc1+ #3208
 | Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
 | Call Trace:
 |  <TASK>
 |  dump_stack_lvl+0x88/0xd8
 |  dump_stack+0x15/0x1b
 |  register_lock_class+0x6b3/0x840
 |  ...
 |  test_barrier+0x490/0x14c7
 |  kcsan_selftest+0x47/0xa0
 |  ...

To fix, move the test locks into static storage.

Fixing the above also revealed that lock operations are strengthened on
first use with lockdep enabled, due to lockdep calling out into
non-instrumented files (recall that kernel/locking/lockdep.c is not
instrumented with KCSAN).

Only kcsan_test checks for over-instrumentation of *_lock() operations,
where we can simply "warm up" the test locks to avoid the test case
failing with lockdep.

Reported-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
  • Loading branch information
Marco Elver authored and Paul E. McKenney committed Dec 10, 2021
1 parent bd3d5bd commit a70d36e
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 21 deletions.
37 changes: 23 additions & 14 deletions kernel/kcsan/kcsan_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@ static struct {
long val[8];
} test_struct;
static DEFINE_SEQLOCK(test_seqlock);
static DEFINE_SPINLOCK(test_spinlock);
static DEFINE_MUTEX(test_mutex);

/*
* Helper to avoid compiler optimizing out reads, and to generate source values
Expand Down Expand Up @@ -523,8 +525,6 @@ static void test_barrier_nothreads(struct kunit *test)
struct kcsan_scoped_access *reorder_access = NULL;
#endif
arch_spinlock_t arch_spinlock = __ARCH_SPIN_LOCK_UNLOCKED;
DEFINE_SPINLOCK(spinlock);
DEFINE_MUTEX(mutex);
atomic_t dummy;

KCSAN_TEST_REQUIRES(test, reorder_access != NULL);
Expand All @@ -543,6 +543,15 @@ static void test_barrier_nothreads(struct kunit *test)
#define KCSAN_EXPECT_WRITE_BARRIER(b, o) __KCSAN_EXPECT_BARRIER(KCSAN_ACCESS_WRITE, b, o, #b)
#define KCSAN_EXPECT_RW_BARRIER(b, o) __KCSAN_EXPECT_BARRIER(KCSAN_ACCESS_COMPOUND | KCSAN_ACCESS_WRITE, b, o, #b)

/*
* Lockdep initialization can strengthen certain locking operations due
* to calling into instrumented files; "warm up" our locks.
*/
spin_lock(&test_spinlock);
spin_unlock(&test_spinlock);
mutex_lock(&test_mutex);
mutex_unlock(&test_mutex);

/* Force creating a valid entry in reorder_access first. */
test_var = 0;
while (test_var++ < 1000000 && reorder_access->size != sizeof(test_var))
Expand Down Expand Up @@ -592,10 +601,10 @@ static void test_barrier_nothreads(struct kunit *test)
KCSAN_EXPECT_READ_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var), true);
KCSAN_EXPECT_READ_BARRIER(arch_spin_lock(&arch_spinlock), false);
KCSAN_EXPECT_READ_BARRIER(arch_spin_unlock(&arch_spinlock), true);
KCSAN_EXPECT_READ_BARRIER(spin_lock(&spinlock), false);
KCSAN_EXPECT_READ_BARRIER(spin_unlock(&spinlock), true);
KCSAN_EXPECT_READ_BARRIER(mutex_lock(&mutex), false);
KCSAN_EXPECT_READ_BARRIER(mutex_unlock(&mutex), true);
KCSAN_EXPECT_READ_BARRIER(spin_lock(&test_spinlock), false);
KCSAN_EXPECT_READ_BARRIER(spin_unlock(&test_spinlock), true);
KCSAN_EXPECT_READ_BARRIER(mutex_lock(&test_mutex), false);
KCSAN_EXPECT_READ_BARRIER(mutex_unlock(&test_mutex), true);

KCSAN_EXPECT_WRITE_BARRIER(mb(), true);
KCSAN_EXPECT_WRITE_BARRIER(wmb(), true);
Expand Down Expand Up @@ -638,10 +647,10 @@ static void test_barrier_nothreads(struct kunit *test)
KCSAN_EXPECT_WRITE_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var), true);
KCSAN_EXPECT_WRITE_BARRIER(arch_spin_lock(&arch_spinlock), false);
KCSAN_EXPECT_WRITE_BARRIER(arch_spin_unlock(&arch_spinlock), true);
KCSAN_EXPECT_WRITE_BARRIER(spin_lock(&spinlock), false);
KCSAN_EXPECT_WRITE_BARRIER(spin_unlock(&spinlock), true);
KCSAN_EXPECT_WRITE_BARRIER(mutex_lock(&mutex), false);
KCSAN_EXPECT_WRITE_BARRIER(mutex_unlock(&mutex), true);
KCSAN_EXPECT_WRITE_BARRIER(spin_lock(&test_spinlock), false);
KCSAN_EXPECT_WRITE_BARRIER(spin_unlock(&test_spinlock), true);
KCSAN_EXPECT_WRITE_BARRIER(mutex_lock(&test_mutex), false);
KCSAN_EXPECT_WRITE_BARRIER(mutex_unlock(&test_mutex), true);

KCSAN_EXPECT_RW_BARRIER(mb(), true);
KCSAN_EXPECT_RW_BARRIER(wmb(), true);
Expand Down Expand Up @@ -684,10 +693,10 @@ static void test_barrier_nothreads(struct kunit *test)
KCSAN_EXPECT_RW_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var), true);
KCSAN_EXPECT_RW_BARRIER(arch_spin_lock(&arch_spinlock), false);
KCSAN_EXPECT_RW_BARRIER(arch_spin_unlock(&arch_spinlock), true);
KCSAN_EXPECT_RW_BARRIER(spin_lock(&spinlock), false);
KCSAN_EXPECT_RW_BARRIER(spin_unlock(&spinlock), true);
KCSAN_EXPECT_RW_BARRIER(mutex_lock(&mutex), false);
KCSAN_EXPECT_RW_BARRIER(mutex_unlock(&mutex), true);
KCSAN_EXPECT_RW_BARRIER(spin_lock(&test_spinlock), false);
KCSAN_EXPECT_RW_BARRIER(spin_unlock(&test_spinlock), true);
KCSAN_EXPECT_RW_BARRIER(mutex_lock(&test_mutex), false);
KCSAN_EXPECT_RW_BARRIER(mutex_unlock(&test_mutex), true);

kcsan_nestable_atomic_end();
}
Expand Down
14 changes: 7 additions & 7 deletions kernel/kcsan/selftest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ static bool __init test_matching_access(void)
* positives: simple test to check at boot certain barriers are always properly
* instrumented. See kcsan_test for a more complete test.
*/
static DEFINE_SPINLOCK(test_spinlock);
static bool __init test_barrier(void)
{
#ifdef CONFIG_KCSAN_WEAK_MEMORY
Expand All @@ -122,7 +123,6 @@ static bool __init test_barrier(void)
#endif
bool ret = true;
arch_spinlock_t arch_spinlock = __ARCH_SPIN_LOCK_UNLOCKED;
DEFINE_SPINLOCK(spinlock);
atomic_t dummy;
long test_var;

Expand Down Expand Up @@ -172,8 +172,8 @@ static bool __init test_barrier(void)
KCSAN_CHECK_READ_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var));
arch_spin_lock(&arch_spinlock);
KCSAN_CHECK_READ_BARRIER(arch_spin_unlock(&arch_spinlock));
spin_lock(&spinlock);
KCSAN_CHECK_READ_BARRIER(spin_unlock(&spinlock));
spin_lock(&test_spinlock);
KCSAN_CHECK_READ_BARRIER(spin_unlock(&test_spinlock));

KCSAN_CHECK_WRITE_BARRIER(mb());
KCSAN_CHECK_WRITE_BARRIER(wmb());
Expand Down Expand Up @@ -202,8 +202,8 @@ static bool __init test_barrier(void)
KCSAN_CHECK_WRITE_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var));
arch_spin_lock(&arch_spinlock);
KCSAN_CHECK_WRITE_BARRIER(arch_spin_unlock(&arch_spinlock));
spin_lock(&spinlock);
KCSAN_CHECK_WRITE_BARRIER(spin_unlock(&spinlock));
spin_lock(&test_spinlock);
KCSAN_CHECK_WRITE_BARRIER(spin_unlock(&test_spinlock));

KCSAN_CHECK_RW_BARRIER(mb());
KCSAN_CHECK_RW_BARRIER(wmb());
Expand Down Expand Up @@ -235,8 +235,8 @@ static bool __init test_barrier(void)
KCSAN_CHECK_RW_BARRIER(clear_bit_unlock_is_negative_byte(0, &test_var));
arch_spin_lock(&arch_spinlock);
KCSAN_CHECK_RW_BARRIER(arch_spin_unlock(&arch_spinlock));
spin_lock(&spinlock);
KCSAN_CHECK_RW_BARRIER(spin_unlock(&spinlock));
spin_lock(&test_spinlock);
KCSAN_CHECK_RW_BARRIER(spin_unlock(&test_spinlock));

kcsan_nestable_atomic_end();

Expand Down

0 comments on commit a70d36e

Please sign in to comment.