Richard Henderson <richard.hender...@linaro.org> writes:
> On 4/7/21 11:39 AM, Alex Bennée wrote:
>> Richard Henderson <richard.hender...@linaro.org> writes:
>>
>>> We were incorrectly assuming that only the first byte of an MTE access
>>> is checked against the tags. But per the ARM, unaligned accesses are
>>> pre-decomposed into single-byte accesses. So by the time we reach the
>>> actual MTE check in the ARM pseudocode, all accesses are aligned.
>>>
>>> Therefore, the first failure is always either the first byte of the
>>> access, or the first byte of the granule.
>>>
<snip>
I replicated the original test case as a kunit test:
static void kmalloc_node_oob_span_right(struct kunit *test)
{
char *ptr;
size_t size = 128;
ptr = kmalloc_node(size, GFP_KERNEL, 0);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
KUNIT_EXPECT_KASAN_FAIL(test, *(volatile unsigned long *)(ptr + 124)
= 0);
kfree(ptr);
}
which before this fix triggered:
[ 6.753429] # kmalloc_node_oob_span_right: EXPECTATION FAILED at
lib/test_kasan.c:163
[ 6.753429] Expected ({ do { extern void
__compiletime_assert_337(void) __attribute__((__error__("Unsupported access
size for {READ,WRITE}_ONCE()."))); if (!((sizeof(
fail_data.report_expected) == sizeof(char) ||
sizeof(fail_data.report_expected) == sizeof(short) ||
sizeof(fail_data.report_expected) == sizeof(int) || sizeof(fail_data.repo
rt_expected) == sizeof(long)) || sizeof(fail_data.report_expected) ==
sizeof(long long))) __compiletime_assert_337(); } while (0); (*(const volatile
typeof( _Generic((fail_d
ata.report_expected), char: (char)0, unsigned char: (unsigned char)0, signed
char: (signed char)0, unsigned short: (unsigned short)0, signed short: (signed
short)0, unsigned
int: (unsigned int)0, signed int: (signed int)0, unsigned long: (unsigned
long)0, signed long: (signed long)0, unsigned long long: (unsigned long long)0,
signed long long:
(signed long long)0, default: (fail_data.report_expected))) *
[ 6.759388] not ok 4 - kmalloc_node_oob_span_right
And is OK by the end of the series:
[ 6.587381] The buggy address belongs to the object at ffff000003325800
[ 6.587381] which belongs to the cache kmalloc-128 of size 128
[ 6.588372] The buggy address is located 0 bytes to the right of
[ 6.588372] 128-byte region [ffff000003325800, ffff000003325880)
[ 6.589354] The buggy address belongs to the page:
[ 6.589948] page:(____ptrval____) refcount:1 mapcount:0
mapping:0000000000000000 index:0x0 pfn:0x43325
[ 6.590911] flags: 0x3fffc0000000200(slab)
[ 6.591648] raw: 03fffc0000000200 dead000000000100 dead000000000122
fdff000002401200
[ 6.592346] raw: 0000000000000000 0000000080100010 00000001ffffffff
0000000000000000
[ 6.593007] page dumped because: kasan: bad access detected
[ 6.593532]
[ 6.593775] Memory state around the buggy address:
[ 6.594360] ffff000003325600: f3 f3 f3 f3 f3 f3 f3 f3 fe fe fe fe fe fe
fe fe
[ 6.594991] ffff000003325700: fd fd fd fd fd fd fd fd fe fe fe fe fe fe
fe fe
[ 6.595594] >ffff000003325800: f8 f8 f8 f8 f8 f8 f8 f8 fe fe fe fe fe fe
fe fe
[ 6.596180] ^
[ 6.596714] ffff000003325900: f7 f7 f7 f7 fe fe fe fe fe fe fe fe fe fe
fe fe
[ 6.597309] ffff000003325a00: f4 f4 fe fe fe fe fe fe fe fe fe fe fe fe
fe fe
[ 6.597925]
==================================================================
[ 6.598513] Disabling lock debugging due to kernel taint
[ 6.600353] ok 1 - kmalloc_node_oob_span_right
[ 6.600554] ok 1 - kasan
But it still fails this patch until:
target/arm: Fix unaligned checks for mte_check1, mte_probe1
So I guess that is the one that should have the buglink.
Anyway code wise:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
