* [PATCH -next] mm/kmemleak: annotate a data race in checksum
@ 2020-03-17 13:21 Qian Cai
2020-03-17 13:31 ` Marco Elver
0 siblings, 1 reply; 5+ messages in thread
From: Qian Cai @ 2020-03-17 13:21 UTC (permalink / raw)
To: akpm; +Cc: elver, catalin.marinas, linux-mm, linux-kernel, Qian Cai
Even if KCSAN is disabled for kmemleak, update_checksum() could still
call crc32() (which is outside of kmemleak.c) to dereference
object->pointer. Thus, the value of object->pointer could be accessed
concurrently as noticed by KCSAN,
BUG: KCSAN: data-race in crc32_le_base / do_raw_spin_lock
write to 0xffffb0ea683a7d50 of 4 bytes by task 23575 on cpu 12:
do_raw_spin_lock+0x114/0x200
debug_spin_lock_after at kernel/locking/spinlock_debug.c:91
(inlined by) do_raw_spin_lock at kernel/locking/spinlock_debug.c:115
_raw_spin_lock+0x40/0x50
__handle_mm_fault+0xa9e/0xd00
handle_mm_fault+0xfc/0x2f0
do_page_fault+0x263/0x6f9
page_fault+0x34/0x40
read to 0xffffb0ea683a7d50 of 4 bytes by task 839 on cpu 60:
crc32_le_base+0x67/0x350
crc32_le_base+0x67/0x350:
crc32_body at lib/crc32.c:106
(inlined by) crc32_le_generic at lib/crc32.c:179
(inlined by) crc32_le at lib/crc32.c:197
kmemleak_scan+0x528/0xd90
update_checksum at mm/kmemleak.c:1172
(inlined by) kmemleak_scan at mm/kmemleak.c:1497
kmemleak_scan_thread+0xcc/0xfa
kthread+0x1e0/0x200
ret_from_fork+0x27/0x50
If a shattered value was returned due to a data race, it will be
corrected in the next scan. Thus, annotate it as an intentional data
race using the data_race() macro.
Signed-off-by: Qian Cai <cai@lca.pw>
---
mm/kmemleak.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index e362dc3d2028..d3327756c3a4 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1169,7 +1169,12 @@ static bool update_checksum(struct kmemleak_object *object)
u32 old_csum = object->checksum;
kasan_disable_current();
- object->checksum = crc32(0, (void *)object->pointer, object->size);
+ /*
+ * crc32() will dereference object->pointer. If an unstable value was
+ * returned due to a data race, it will be corrected in the next scan.
+ */
+ object->checksum = data_race(crc32(0, (void *)object->pointer,
+ object->size));
kasan_enable_current();
return object->checksum != old_csum;
--
2.21.0 (Apple Git-122.2)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next] mm/kmemleak: annotate a data race in checksum
2020-03-17 13:21 [PATCH -next] mm/kmemleak: annotate a data race in checksum Qian Cai
@ 2020-03-17 13:31 ` Marco Elver
2020-03-17 13:42 ` Qian Cai
0 siblings, 1 reply; 5+ messages in thread
From: Marco Elver @ 2020-03-17 13:31 UTC (permalink / raw)
To: Qian Cai
Cc: Andrew Morton, catalin.marinas, Linux Memory Management List, LKML
On Tue, 17 Mar 2020 at 14:22, Qian Cai <cai@lca.pw> wrote:
>
> Even if KCSAN is disabled for kmemleak, update_checksum() could still
> call crc32() (which is outside of kmemleak.c) to dereference
> object->pointer. Thus, the value of object->pointer could be accessed
> concurrently as noticed by KCSAN,
>
> BUG: KCSAN: data-race in crc32_le_base / do_raw_spin_lock
>
> write to 0xffffb0ea683a7d50 of 4 bytes by task 23575 on cpu 12:
> do_raw_spin_lock+0x114/0x200
> debug_spin_lock_after at kernel/locking/spinlock_debug.c:91
> (inlined by) do_raw_spin_lock at kernel/locking/spinlock_debug.c:115
> _raw_spin_lock+0x40/0x50
> __handle_mm_fault+0xa9e/0xd00
> handle_mm_fault+0xfc/0x2f0
> do_page_fault+0x263/0x6f9
> page_fault+0x34/0x40
>
> read to 0xffffb0ea683a7d50 of 4 bytes by task 839 on cpu 60:
> crc32_le_base+0x67/0x350
> crc32_le_base+0x67/0x350:
> crc32_body at lib/crc32.c:106
> (inlined by) crc32_le_generic at lib/crc32.c:179
> (inlined by) crc32_le at lib/crc32.c:197
> kmemleak_scan+0x528/0xd90
> update_checksum at mm/kmemleak.c:1172
> (inlined by) kmemleak_scan at mm/kmemleak.c:1497
> kmemleak_scan_thread+0xcc/0xfa
> kthread+0x1e0/0x200
> ret_from_fork+0x27/0x50
>
> If a shattered value was returned due to a data race, it will be
> corrected in the next scan. Thus, annotate it as an intentional data
> race using the data_race() macro.
>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> mm/kmemleak.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index e362dc3d2028..d3327756c3a4 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1169,7 +1169,12 @@ static bool update_checksum(struct kmemleak_object *object)
> u32 old_csum = object->checksum;
>
> kasan_disable_current();
Suggested:
+ kcsan_disable_current();
> - object->checksum = crc32(0, (void *)object->pointer, object->size);
> + /*
> + * crc32() will dereference object->pointer. If an unstable value was
> + * returned due to a data race, it will be corrected in the next scan.
> + */
> + object->checksum = data_race(crc32(0, (void *)object->pointer,
> + object->size));
This will work with the default config, because for word-sized-aligned
writes no marking is enforced. But this will still cause a data race
if the write is e.g. due to a memcpy.
There are already markers for KASAN around, so the most reliable thing
is to just disable KCSAN in this region.
> kasan_enable_current();
Suggested:
+ kcsan_enable_current();
Thanks,
-- Marco
> return object->checksum != old_csum;
> --
> 2.21.0 (Apple Git-122.2)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] mm/kmemleak: annotate a data race in checksum
2020-03-17 13:31 ` Marco Elver
@ 2020-03-17 13:42 ` Qian Cai
0 siblings, 0 replies; 5+ messages in thread
From: Qian Cai @ 2020-03-17 13:42 UTC (permalink / raw)
To: Marco Elver
Cc: Andrew Morton, Catalin Marinas, Linux Memory Management List, LKML
> On Mar 17, 2020, at 9:31 AM, Marco Elver <elver@google.com> wrote:
>
> On Tue, 17 Mar 2020 at 14:22, Qian Cai <cai@lca.pw> wrote:
>>
>> Even if KCSAN is disabled for kmemleak, update_checksum() could still
>> call crc32() (which is outside of kmemleak.c) to dereference
>> object->pointer. Thus, the value of object->pointer could be accessed
>> concurrently as noticed by KCSAN,
>>
>> BUG: KCSAN: data-race in crc32_le_base / do_raw_spin_lock
>>
>> write to 0xffffb0ea683a7d50 of 4 bytes by task 23575 on cpu 12:
>> do_raw_spin_lock+0x114/0x200
>> debug_spin_lock_after at kernel/locking/spinlock_debug.c:91
>> (inlined by) do_raw_spin_lock at kernel/locking/spinlock_debug.c:115
>> _raw_spin_lock+0x40/0x50
>> __handle_mm_fault+0xa9e/0xd00
>> handle_mm_fault+0xfc/0x2f0
>> do_page_fault+0x263/0x6f9
>> page_fault+0x34/0x40
>>
>> read to 0xffffb0ea683a7d50 of 4 bytes by task 839 on cpu 60:
>> crc32_le_base+0x67/0x350
>> crc32_le_base+0x67/0x350:
>> crc32_body at lib/crc32.c:106
>> (inlined by) crc32_le_generic at lib/crc32.c:179
>> (inlined by) crc32_le at lib/crc32.c:197
>> kmemleak_scan+0x528/0xd90
>> update_checksum at mm/kmemleak.c:1172
>> (inlined by) kmemleak_scan at mm/kmemleak.c:1497
>> kmemleak_scan_thread+0xcc/0xfa
>> kthread+0x1e0/0x200
>> ret_from_fork+0x27/0x50
>>
>> If a shattered value was returned due to a data race, it will be
>> corrected in the next scan. Thus, annotate it as an intentional data
>> race using the data_race() macro.
>>
>> Signed-off-by: Qian Cai <cai@lca.pw>
>> ---
>> mm/kmemleak.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index e362dc3d2028..d3327756c3a4 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -1169,7 +1169,12 @@ static bool update_checksum(struct kmemleak_object *object)
>> u32 old_csum = object->checksum;
>>
>> kasan_disable_current();
>
> Suggested:
> + kcsan_disable_current();
>
>> - object->checksum = crc32(0, (void *)object->pointer, object->size);
>> + /*
>> + * crc32() will dereference object->pointer. If an unstable value was
>> + * returned due to a data race, it will be corrected in the next scan.
>> + */
>> + object->checksum = data_race(crc32(0, (void *)object->pointer,
>> + object->size));
>
> This will work with the default config, because for word-sized-aligned
> writes no marking is enforced. But this will still cause a data race
> if the write is e.g. due to a memcpy.
I saw this spla atmt but just decided to reuse an old one to save some time.
Looks like that "head->func = func;” not aligned.
[77392.095571][ T839] BUG: KCSAN: data-race in call_rcu / crc32_le_base
[77392.102066][ T839]
[77392.104297][ T839] write to 0xffff898ea73a8748 of 8 bytes by task 114682 on cpu 79:
[77392.112111][ T839] call_rcu+0xe8/0x4b0
__call_rcu at kernel/rcu/tree.c:2701
(inlined by) call_rcu at kernel/rcu/tree.c:2777
[77392.116084][ T839] __fput+0x23a/0x3d0
[77392.119970][ T839] ____fput+0x1e/0x30
[77392.123852][ T839] task_work_run+0xba/0x120
[77392.128257][ T839] do_syscall_64+0x7d7/0xb05
[77392.132753][ T839] entry_SYSCALL_64_after_hwframe+0x49/0xb3
[77392.138544][ T839]
[77392.140760][ T839] INFO: lockdep is turned off.
[77392.145478][ T839] irq event stamp: 0
[77392.149270][ T839] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[77392.156307][ T839] hardirqs last disabled at (0): [<ffffffffb0ab4d42>] copy_process+0x1122/0x3240
[77392.165348][ T839] softirqs last enabled at (0): [<ffffffffb0ab4d42>] copy_process+0x1122/0x3240
[77392.174384][ T839] softirqs last disabled at (0): [<0000000000000000>] 0x0
[77392.181405][ T839]
[77392.183625][ T839] read to 0xffff898ea73a8748 of 4 bytes by task 839 on cpu 46:
[77392.191088][ T839] crc32_le_base+0x67/0x350
[77392.195498][ T839] kmemleak_scan+0x3ee/0x9f0
[77392.199992][ T839] kmemleak_scan_thread+0x9f/0xc4
[77392.204921][ T839] kthread+0x1cd/0x1f0
[77392.208894][ T839] ret_from_fork+0x27/0x50
>
> There are already markers for KASAN around, so the most reliable thing
> is to just disable KCSAN in this region.
OK, I’ll test that a bit first.
>
>> kasan_enable_current();
>
> Suggested:
> + kcsan_enable_current();
>
> Thanks,
> -- Marco
>
>> return object->checksum != old_csum;
>> --
>> 2.21.0 (Apple Git-122.2)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH -next] mm/kmemleak: annotate a data race in checksum
@ 2020-02-11 16:24 Qian Cai
2020-02-11 16:51 ` Catalin Marinas
0 siblings, 1 reply; 5+ messages in thread
From: Qian Cai @ 2020-02-11 16:24 UTC (permalink / raw)
To: akpm; +Cc: elver, catalin.marinas, linux-mm, linux-kernel, Qian Cai
The value of object->pointer could be accessed concurrently as noticed
by KCSAN,
BUG: KCSAN: data-race in crc32_le_base / do_raw_spin_lock
write to 0xffffb0ea683a7d50 of 4 bytes by task 23575 on cpu 12:
do_raw_spin_lock+0x114/0x200
debug_spin_lock_after at kernel/locking/spinlock_debug.c:91
(inlined by) do_raw_spin_lock at kernel/locking/spinlock_debug.c:115
_raw_spin_lock+0x40/0x50
__handle_mm_fault+0xa9e/0xd00
handle_mm_fault+0xfc/0x2f0
do_page_fault+0x263/0x6f9
page_fault+0x34/0x40
read to 0xffffb0ea683a7d50 of 4 bytes by task 839 on cpu 60:
crc32_le_base+0x67/0x350
crc32_le_base+0x67/0x350:
crc32_body at lib/crc32.c:106
(inlined by) crc32_le_generic at lib/crc32.c:179
(inlined by) crc32_le at lib/crc32.c:197
kmemleak_scan+0x528/0xd90
update_checksum at mm/kmemleak.c:1172
(inlined by) kmemleak_scan at mm/kmemleak.c:1497
kmemleak_scan_thread+0xcc/0xfa
kthread+0x1e0/0x200
ret_from_fork+0x27/0x50
Reported by Kernel Concurrency Sanitizer on:
CPU: 60 PID: 839 Comm: kmemleak Tainted: G W L 5.5.0-next-20200210+ #3
Hardware name: HPE ProLiant DL385 Gen10/ProLiant DL385 Gen10, BIOS A40 07/10/2019
crc32() will dereference object->pointer. If a shattered value was
returned due to a data race, it will be corrected in the next scan.
Thus, annotate it as an intentional data race using the data_race()
macro.
Signed-off-by: Qian Cai <cai@lca.pw>
---
mm/kmemleak.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 3a4259eeb5a0..25b4bcc32b5f 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1169,7 +1169,12 @@ static bool update_checksum(struct kmemleak_object *object)
u32 old_csum = object->checksum;
kasan_disable_current();
- object->checksum = crc32(0, (void *)object->pointer, object->size);
+ /*
+ * crc32() will dereference object->pointer. If an unstable value was
+ * returned due to a data race, it will be corrected in the next scan.
+ */
+ object->checksum = data_race(crc32(0, (void *)object->pointer,
+ object->size));
kasan_enable_current();
return object->checksum != old_csum;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next] mm/kmemleak: annotate a data race in checksum
2020-02-11 16:24 Qian Cai
@ 2020-02-11 16:51 ` Catalin Marinas
0 siblings, 0 replies; 5+ messages in thread
From: Catalin Marinas @ 2020-02-11 16:51 UTC (permalink / raw)
To: Qian Cai; +Cc: akpm, elver, linux-mm, linux-kernel
On Tue, Feb 11, 2020 at 11:24:05AM -0500, Qian Cai wrote:
> The value of object->pointer could be accessed concurrently as noticed
> by KCSAN,
>
> BUG: KCSAN: data-race in crc32_le_base / do_raw_spin_lock
>
> write to 0xffffb0ea683a7d50 of 4 bytes by task 23575 on cpu 12:
> do_raw_spin_lock+0x114/0x200
> debug_spin_lock_after at kernel/locking/spinlock_debug.c:91
> (inlined by) do_raw_spin_lock at kernel/locking/spinlock_debug.c:115
> _raw_spin_lock+0x40/0x50
> __handle_mm_fault+0xa9e/0xd00
> handle_mm_fault+0xfc/0x2f0
> do_page_fault+0x263/0x6f9
> page_fault+0x34/0x40
>
> read to 0xffffb0ea683a7d50 of 4 bytes by task 839 on cpu 60:
> crc32_le_base+0x67/0x350
> crc32_le_base+0x67/0x350:
> crc32_body at lib/crc32.c:106
> (inlined by) crc32_le_generic at lib/crc32.c:179
> (inlined by) crc32_le at lib/crc32.c:197
> kmemleak_scan+0x528/0xd90
> update_checksum at mm/kmemleak.c:1172
> (inlined by) kmemleak_scan at mm/kmemleak.c:1497
> kmemleak_scan_thread+0xcc/0xfa
> kthread+0x1e0/0x200
> ret_from_fork+0x27/0x50
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 60 PID: 839 Comm: kmemleak Tainted: G W L 5.5.0-next-20200210+ #3
> Hardware name: HPE ProLiant DL385 Gen10/ProLiant DL385 Gen10, BIOS A40 07/10/2019
>
> crc32() will dereference object->pointer. If a shattered value was
> returned due to a data race, it will be corrected in the next scan.
> Thus, annotate it as an intentional data race using the data_race()
> macro.
>
> Signed-off-by: Qian Cai <cai@lca.pw>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-17 13:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 13:21 [PATCH -next] mm/kmemleak: annotate a data race in checksum Qian Cai
2020-03-17 13:31 ` Marco Elver
2020-03-17 13:42 ` Qian Cai
-- strict thread matches above, loose matches on Subject: below --
2020-02-11 16:24 Qian Cai
2020-02-11 16:51 ` Catalin Marinas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).