Commit 6f2d9819 authored by Mark Rutland's avatar Mark Rutland Committed by Paul E. McKenney
Browse files

kcsan: Simplify value change detection 



In kcsan_setup_watchpoint() we store snapshots of a watched value into a
union of u8/u16/u32/u64 sized fields, modify this in place using a
consistent field, then later check for any changes via the u64 field.

We can achieve the safe effect more simply by always treating the field
as a u64, as smaller values will be zero-extended. As the values are
zero-extended, we don't need to truncate the access_mask when we apply
it, and can always apply the full 64-bit access_mask to the 64-bit
value.

Finally, we can store the two snapshots and calculated difference
separately, which makes the code a little easier to read, and will
permit reporting the old/new values in subsequent patches.

There should be no functional change as a result of this patch.

Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Signed-off-by: default avatarMarco Elver <elver@google.com>
Signed-off-by: default avatarPaul E. McKenney <paulmck@kernel.org>
parent ea048464

kernel/kcsan/core.c

+16 −24
Original line number Diff line number Diff line
@@ -407,12 +407,7 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type) 
	const bool is_write = (type & KCSAN_ACCESS_WRITE) != 0;

	const bool is_assert = (type & KCSAN_ACCESS_ASSERT) != 0;

	atomic_long_t *watchpoint;

	union {

		u8 _1;

		u16 _2;

		u32 _4;

		u64 _8;

	} expect_value;

	u64 old, new, diff;

	unsigned long access_mask;

	enum kcsan_value_change value_change = KCSAN_VALUE_CHANGE_MAYBE;

	unsigned long ua_flags = user_access_save();
@@ -468,19 +463,19 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type) 
	 * Read the current value, to later check and infer a race if the data

	 * was modified via a non-instrumented access, e.g. from a device.

	 */

	expect_value._8 = 0;

	old = 0;

	switch (size) {

	case 1:

		expect_value._1 = READ_ONCE(*(const u8 *)ptr);

		old = READ_ONCE(*(const u8 *)ptr);

		break;

	case 2:

		expect_value._2 = READ_ONCE(*(const u16 *)ptr);

		old = READ_ONCE(*(const u16 *)ptr);

		break;

	case 4:

		expect_value._4 = READ_ONCE(*(const u32 *)ptr);

		old = READ_ONCE(*(const u32 *)ptr);

		break;

	case 8:

		expect_value._8 = READ_ONCE(*(const u64 *)ptr);

		old = READ_ONCE(*(const u64 *)ptr);

		break;

	default:

		break; /* ignore; we do not diff the values */
@@ -506,33 +501,30 @@ kcsan_setup_watchpoint(const volatile void *ptr, size_t size, int type) 
	 * racy access.

	 */

	access_mask = get_ctx()->access_mask;

	new = 0;

	switch (size) {

	case 1:

		expect_value._1 ^= READ_ONCE(*(const u8 *)ptr);

		if (access_mask)

			expect_value._1 &= (u8)access_mask;

		new = READ_ONCE(*(const u8 *)ptr);

		break;

	case 2:

		expect_value._2 ^= READ_ONCE(*(const u16 *)ptr);

		if (access_mask)

			expect_value._2 &= (u16)access_mask;

		new = READ_ONCE(*(const u16 *)ptr);

		break;

	case 4:

		expect_value._4 ^= READ_ONCE(*(const u32 *)ptr);

		if (access_mask)

			expect_value._4 &= (u32)access_mask;

		new = READ_ONCE(*(const u32 *)ptr);

		break;

	case 8:

		expect_value._8 ^= READ_ONCE(*(const u64 *)ptr);

		if (access_mask)

			expect_value._8 &= (u64)access_mask;

		new = READ_ONCE(*(const u64 *)ptr);

		break;

	default:

		break; /* ignore; we do not diff the values */

	}



	diff = old ^ new;

	if (access_mask)

		diff &= access_mask;



	/* Were we able to observe a value-change? */

	if (expect_value._8 != 0)

	if (diff != 0)

		value_change = KCSAN_VALUE_CHANGE_TRUE;



	/* Check if this access raced with another. */