Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On 7/21/19 4:49 PM, Luis Henriques wrote: > Waiman Long writes: > >> On 7/20/19 4:41 AM, Luis Henriques wrote: >>> "Linus Torvalds" writes: >>> On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: > This patch shouldn't change the behavior of the rwsem code. The code > only access data within the rw_semaphore structures. I don't know why it > will cause a KASAN error. I will have to reproduce it and figure out > exactly which statement is doing the invalid access. The stack traces should show line numbers if you run them through scripts/decode_stacktrace.sh. You need to have debug info enabled for that, though. Luis? Linus >>> Yep, sure. And I should have done this in the initial report. It's a >>> different trace, I had to recompile the kernel. >>> >>> (I'm also adding Jeff to the CC list.) >>> >>> Cheers, >> Thanks for the information. I think I know where the problem is. Would >> you mind applying the attached patch to see if it can fix the KASAN error. > Yep, that seems to work -- I can't reproduce the error anymore (and > sorry for the delay). Thanks! And feel free to add my Tested-by. > > Cheers, Thanks for the testing. I will post the official patch tomorrow. Cheers, Longman
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
Waiman Long writes: > On 7/20/19 4:41 AM, Luis Henriques wrote: >> "Linus Torvalds" writes: >> >>> On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: This patch shouldn't change the behavior of the rwsem code. The code only access data within the rw_semaphore structures. I don't know why it will cause a KASAN error. I will have to reproduce it and figure out exactly which statement is doing the invalid access. >>> The stack traces should show line numbers if you run them through >>> scripts/decode_stacktrace.sh. >>> >>> You need to have debug info enabled for that, though. >>> >>> Luis? >>> >>> Linus >> Yep, sure. And I should have done this in the initial report. It's a >> different trace, I had to recompile the kernel. >> >> (I'm also adding Jeff to the CC list.) >> >> Cheers, > > Thanks for the information. I think I know where the problem is. Would > you mind applying the attached patch to see if it can fix the KASAN error. Yep, that seems to work -- I can't reproduce the error anymore (and sorry for the delay). Thanks! And feel free to add my Tested-by. Cheers, -- Luis
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On 7/20/19 4:41 AM, Luis Henriques wrote:
> "Linus Torvalds" writes:
>
>> On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote:
>>> This patch shouldn't change the behavior of the rwsem code. The code
>>> only access data within the rw_semaphore structures. I don't know why it
>>> will cause a KASAN error. I will have to reproduce it and figure out
>>> exactly which statement is doing the invalid access.
>> The stack traces should show line numbers if you run them through
>> scripts/decode_stacktrace.sh.
>>
>> You need to have debug info enabled for that, though.
>>
>> Luis?
>>
>> Linus
> Yep, sure. And I should have done this in the initial report. It's a
> different trace, I had to recompile the kernel.
>
> (I'm also adding Jeff to the CC list.)
>
> Cheers,
Thanks for the information. I think I know where the problem is. Would
you mind applying the attached patch to see if it can fix the KASAN error.
Thanks,
Longman
>From 445dc58add71f976c20359568d370f5059d30f77 Mon Sep 17 00:00:00 2001
From: Waiman Long
Date: Sat, 20 Jul 2019 10:45:39 -0400
Subject: [PATCH] locking/rwsem: Don't call owner_on_cpu() on read-owner
For writer, the owner value is cleared on unlock. For reader, it is
left intact on unlock for providing better debugging aid on crash dump
and the unlock of one reader may not mean the lock is free.
As a result, the owner_on_cpu() shouldn't be used on read-owner
as the task pointer value may not be valid and it might have
been freed. That is the case in rwsem_spin_on_owner(), but not in
rwsem_can_spin_on_owner(). This can lead to use-after-free error from
KASAN. For example,
BUG: KASAN: use-after-free in rwsem_down_write_slowpath
(/home/miguel/kernel/linux/kernel/locking/rwsem.c:669
/home/miguel/kernel/linux/kernel/locking/rwsem.c:1125)
Fix this by checking for RWSEM_READER_OWNED flag before calling
owner_on_cpu().
Fixes: 94a9717b3c40e ("locking/rwsem: Make rwsem->owner an atomic_long_t")
Signed-off-by: Waiman Long
---
kernel/locking/rwsem.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
index 37524a47f002..bc91aacaab58 100644
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -666,7 +666,11 @@ static inline bool rwsem_can_spin_on_owner(struct rw_semaphore *sem,
preempt_disable();
rcu_read_lock();
owner = rwsem_owner_flags(sem, );
- if ((flags & nonspinnable) || (owner && !owner_on_cpu(owner)))
+ /*
+ * Don't check the read-owner as the entry may be stale.
+ */
+ if ((flags & nonspinnable) ||
+ (owner && !(flags & RWSEM_READER_OWNED) && !owner_on_cpu(owner)))
ret = false;
rcu_read_unlock();
preempt_enable();
--
2.18.1
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On Sat, Jul 20, 2019 at 09:41:05AM +0100, Luis Henriques wrote: > [ 39.801179] > == > [ 39.801973] BUG: KASAN: use-after-free in rwsem_down_write_slowpath > (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 > /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) That's rwsem_can_spin_on_owner(), specifically line 669 seems to suggest owner_on_cpu(). So we'd somehow have a dead owner; I'm not immediately seeing how that can happen.
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
Luis Henriques writes: > Luis Henriques writes: > >> "Linus Torvalds" writes: >> >>> On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: This patch shouldn't change the behavior of the rwsem code. The code only access data within the rw_semaphore structures. I don't know why it will cause a KASAN error. I will have to reproduce it and figure out exactly which statement is doing the invalid access. >>> >>> The stack traces should show line numbers if you run them through >>> scripts/decode_stacktrace.sh. >>> >>> You need to have debug info enabled for that, though. >>> >>> Luis? >>> >>> Linus >> >> Yep, sure. And I should have done this in the initial report. It's a >> different trace, I had to recompile the kernel. >> >> (I'm also adding Jeff to the CC list.) >> > > Ah, and I also managed to reproduce this on btrfs so I guess this rules > out a bug in the filesystem code. Just another detail (before I go completely offline until tomorrow evening): in the btrfs case I'm seeing the bug on the rwsem_down_read_slowpath path, not on rwsem_down_write_slowpath. But it seems to be on the same place (i.e. rwsem_can_spin_on_owner). Cheers, -- Luis
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
Luis Henriques writes: > "Linus Torvalds" writes: > >> On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: >>> >>> This patch shouldn't change the behavior of the rwsem code. The code >>> only access data within the rw_semaphore structures. I don't know why it >>> will cause a KASAN error. I will have to reproduce it and figure out >>> exactly which statement is doing the invalid access. >> >> The stack traces should show line numbers if you run them through >> scripts/decode_stacktrace.sh. >> >> You need to have debug info enabled for that, though. >> >> Luis? >> >> Linus > > Yep, sure. And I should have done this in the initial report. It's a > different trace, I had to recompile the kernel. > > (I'm also adding Jeff to the CC list.) > Ah, and I also managed to reproduce this on btrfs so I guess this rules out a bug in the filesystem code. Cheers, -- Luis
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
"Linus Torvalds" writes: > On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: >> >> This patch shouldn't change the behavior of the rwsem code. The code >> only access data within the rw_semaphore structures. I don't know why it >> will cause a KASAN error. I will have to reproduce it and figure out >> exactly which statement is doing the invalid access. > > The stack traces should show line numbers if you run them through > scripts/decode_stacktrace.sh. > > You need to have debug info enabled for that, though. > > Luis? > > Linus Yep, sure. And I should have done this in the initial report. It's a different trace, I had to recompile the kernel. (I'm also adding Jeff to the CC list.) Cheers, -- Luis [ 39.801179] == [ 39.801973] BUG: KASAN: use-after-free in rwsem_down_write_slowpath (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) [ 39.802733] Read of size 4 at addr 8881f1f65138 by task xfs_io/2145 [ 39.803598] CPU: 0 PID: 2145 Comm: xfs_io Not tainted 5.2.0+ #460 [ 39.803600] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58-prebuilt.qemu.org 04/01/2014 [ 39.803602] Call Trace: [ 39.803609] dump_stack (/home/miguel/kernel/linux/lib/dump_stack.c:115) [ 39.803615] print_address_description (/home/miguel/kernel/linux/mm/kasan/report.c:352) [ 39.803618] ? rwsem_down_write_slowpath (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) [ 39.803621] ? rwsem_down_write_slowpath (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) [ 39.803624] __kasan_report.cold (/home/miguel/kernel/linux/mm/kasan/report.c:483) [ 39.803629] ? rwsem_down_write_slowpath (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) [ 39.803633] kasan_report (/home/miguel/kernel/linux/./arch/x86/include/asm/smap.h:69 /home/miguel/kernel/linux/mm/kasan/common.c:613) [ 39.803636] rwsem_down_write_slowpath (/home/miguel/kernel/linux/kernel/locking/rwsem.c:669 /home/miguel/kernel/linux/kernel/locking/rwsem.c:1125) [ 39.803641] ? __ceph_caps_issued_mask (/home/miguel/kernel/linux/fs/ceph/caps.c:914) [ 39.803644] ? find_held_lock (/home/miguel/kernel/linux/kernel/locking/lockdep.c:4004) [ 39.803649] ? __ceph_do_getattr (/home/miguel/kernel/linux/fs/ceph/inode.c:2246) [ 39.803653] ? down_read_non_owner (/home/miguel/kernel/linux/kernel/locking/rwsem.c:1116) [ 39.803658] ? do_raw_spin_unlock (/home/miguel/kernel/linux/./include/linux/compiler.h:218 /home/miguel/kernel/linux/./include/asm-generic/qspinlock.h:94 /home/miguel/kernel/linux/kernel/locking/spinlock_debug.c:139) [ 39.803663] ? _raw_spin_unlock (/home/miguel/kernel/linux/kernel/locking/spinlock.c:184) [ 39.803667] ? __lock_acquire.isra.0 (/home/miguel/kernel/linux/kernel/locking/lockdep.c:3884) [ 39.803674] ? path_openat (/home/miguel/kernel/linux/fs/namei.c:3322 /home/miguel/kernel/linux/fs/namei.c:3533) [ 39.803680] ? down_write (/home/miguel/kernel/linux/kernel/locking/rwsem.c:1486) [ 39.803683] down_write (/home/miguel/kernel/linux/kernel/locking/rwsem.c:1486) [ 39.803687] ? down_read_killable (/home/miguel/kernel/linux/kernel/locking/rwsem.c:1482) [ 39.803690] ? __sb_start_write (/home/miguel/kernel/linux/./include/linux/compiler.h:194 /home/miguel/kernel/linux/./include/linux/rcu_sync.h:38 /home/miguel/kernel/linux/./include/linux/percpu-rwsem.h:52 /home/miguel/kernel/linux/fs/super.c:1608) [ 39.803694] ? __mnt_want_write (/home/miguel/kernel/linux/fs/namespace.c:253 /home/miguel/kernel/linux/fs/namespace.c:297 /home/miguel/kernel/linux/fs/namespace.c:337) [ 39.803699] path_openat (/home/miguel/kernel/linux/fs/namei.c:3322 /home/miguel/kernel/linux/fs/namei.c:3533) [ 39.803706] ? path_mountpoint (/home/miguel/kernel/linux/fs/namei.c:3518) [ 39.803711] ? __is_insn_slot_addr (/home/miguel/kernel/linux/kernel/kprobes.c:291) [ 39.803716] ? kernel_text_address (/home/miguel/kernel/linux/kernel/extable.c:113) [ 39.803719] ? __kernel_text_address (/home/miguel/kernel/linux/kernel/extable.c:95) [ 39.803724] ? unwind_get_return_address (/home/miguel/kernel/linux/arch/x86/kernel/unwind_orc.c:311 /home/miguel/kernel/linux/arch/x86/kernel/unwind_orc.c:306) [ 39.803727] ? swiotlb_map.cold (/home/miguel/kernel/linux/kernel/stacktrace.c:83) [ 39.803730] ? arch_stack_walk (/home/miguel/kernel/linux/arch/x86/kernel/stacktrace.c:26) [ 39.803735] do_filp_open (/home/miguel/kernel/linux/fs/namei.c:3563) [ 39.803739] ? may_open_dev (/home/miguel/kernel/linux/fs/namei.c:3557) [ 39.803746] ? __alloc_fd (/home/miguel/kernel/linux/fs/file.c:536) [ 39.803749] ? lock_downgrade
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On 7/19/19 3:45 PM, Luis Henriques wrote:
> Waiman Long writes:
>
>> On 7/19/19 2:45 PM, Luis Henriques wrote:
>>> On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
The rwsem->owner contains not just the task structure pointer, it also
holds some flags for storing the current state of the rwsem. Some of
the flags may have to be atomically updated. To reflect the new reality,
the owner is now changed to an atomic_long_t type.
New helper functions are added to properly separate out the task
structure pointer and the embedded flags.
>>> I started seeing KASAN use-after-free with current master, and a bisect
>>> showed me that this commit 94a9717b3c40 ("locking/rwsem: Make
>>> rwsem->owner an atomic_long_t") was the problem. Does it ring any
>>> bells? I can easily reproduce it with xfstests (generic/464).
>>>
>>> Cheers,
>>> --
>>> Luís
>> This patch shouldn't change the behavior of the rwsem code. The code
>> only access data within the rw_semaphore structures. I don't know why it
>> will cause a KASAN error. I will have to reproduce it and figure out
>> exactly which statement is doing the invalid access.
> Yeah, screwing the bisection is something I've done in the past so I may
> have got the wrong commit. Another detail is that I was running
> xfstests against CephFS, I didn't tried with any other filesystem. I
> can try to reproduce with btrfs or xfs next week.
>
> Cheers,
Oh, I don't have a CephFS setup. Will you use the
scripts/decode_stacktrace.sh to find what line number is the offending
statement? That will help in figuring out what has gone wrong.
Anyway, it seems like a structure that include a rwsem is freed while
another cpu is still waiting to acquire the lock. It is probably a
hidden bug in the filesystem code somewhere that the recent changes in
rwsem behavior make it easier for the problem to show up.
Cheers,
Longman
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On Fri, Jul 19, 2019 at 12:32 PM Waiman Long wrote: > > This patch shouldn't change the behavior of the rwsem code. The code > only access data within the rw_semaphore structures. I don't know why it > will cause a KASAN error. I will have to reproduce it and figure out > exactly which statement is doing the invalid access. The stack traces should show line numbers if you run them through scripts/decode_stacktrace.sh. You need to have debug info enabled for that, though. Luis? Linus
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
Waiman Long writes:
> On 7/19/19 2:45 PM, Luis Henriques wrote:
>> On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
>>> The rwsem->owner contains not just the task structure pointer, it also
>>> holds some flags for storing the current state of the rwsem. Some of
>>> the flags may have to be atomically updated. To reflect the new reality,
>>> the owner is now changed to an atomic_long_t type.
>>>
>>> New helper functions are added to properly separate out the task
>>> structure pointer and the embedded flags.
>> I started seeing KASAN use-after-free with current master, and a bisect
>> showed me that this commit 94a9717b3c40 ("locking/rwsem: Make
>> rwsem->owner an atomic_long_t") was the problem. Does it ring any
>> bells? I can easily reproduce it with xfstests (generic/464).
>>
>> Cheers,
>> --
>> Luís
>
> This patch shouldn't change the behavior of the rwsem code. The code
> only access data within the rw_semaphore structures. I don't know why it
> will cause a KASAN error. I will have to reproduce it and figure out
> exactly which statement is doing the invalid access.
Yeah, screwing the bisection is something I've done in the past so I may
have got the wrong commit. Another detail is that I was running
xfstests against CephFS, I didn't tried with any other filesystem. I
can try to reproduce with btrfs or xfs next week.
Cheers,
--
Luis
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On 7/19/19 2:45 PM, Luis Henriques wrote:
> On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
>> The rwsem->owner contains not just the task structure pointer, it also
>> holds some flags for storing the current state of the rwsem. Some of
>> the flags may have to be atomically updated. To reflect the new reality,
>> the owner is now changed to an atomic_long_t type.
>>
>> New helper functions are added to properly separate out the task
>> structure pointer and the embedded flags.
> I started seeing KASAN use-after-free with current master, and a bisect
> showed me that this commit 94a9717b3c40 ("locking/rwsem: Make
> rwsem->owner an atomic_long_t") was the problem. Does it ring any
> bells? I can easily reproduce it with xfstests (generic/464).
>
> Cheers,
> --
> Luís
This patch shouldn't change the behavior of the rwsem code. The code
only access data within the rw_semaphore structures. I don't know why it
will cause a KASAN error. I will have to reproduce it and figure out
exactly which statement is doing the invalid access.
Thanks,
Longman
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
> The rwsem->owner contains not just the task structure pointer, it also
> holds some flags for storing the current state of the rwsem. Some of
> the flags may have to be atomically updated. To reflect the new reality,
> the owner is now changed to an atomic_long_t type.
>
> New helper functions are added to properly separate out the task
> structure pointer and the embedded flags.
I started seeing KASAN use-after-free with current master, and a bisect
showed me that this commit 94a9717b3c40 ("locking/rwsem: Make
rwsem->owner an atomic_long_t") was the problem. Does it ring any
bells? I can easily reproduce it with xfstests (generic/464).
Cheers,
--
Luís
[ 6380.820179] run fstests generic/464 at 2019-07-19 12:04:05
[ 6381.504693] libceph: mon0 (1)192.168.155.1:40786 session established
[ 6381.506790] libceph: client4572 fsid 86b39301-7192-4052-8427-a241af35a591
[ 6381.618830] libceph: mon0 (1)192.168.155.1:40786 session established
[ 6381.619993] libceph: client4573 fsid 86b39301-7192-4052-8427-a241af35a591
[ 6384.464561]
==
[ 6384.466165] BUG: KASAN: use-after-free in
rwsem_down_write_slowpath+0x67d/0x8a0
[ 6384.468288] Read of size 4 at addr 8881d5dc9478 by task xfs_io/17238
[ 6384.469545] CPU: 1 PID: 17238 Comm: xfs_io Not tainted 5.2.0+ #444
[ 6384.469550] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.12.1-0-ga5cab58-prebuilt.qemu.org 04/01/2014
[ 6384.469554] Call Trace:
[ 6384.469563] dump_stack+0x5b/0x90
[ 6384.469569] print_address_description+0x6f/0x332
[ 6384.469573] ? rwsem_down_write_slowpath+0x67d/0x8a0
[ 6384.469575] ? rwsem_down_write_slowpath+0x67d/0x8a0
[ 6384.469579] __kasan_report.cold+0x1a/0x3e
[ 6384.469583] ? rwsem_down_write_slowpath+0x67d/0x8a0
[ 6384.469588] kasan_report+0xe/0x12
[ 6384.469591] rwsem_down_write_slowpath+0x67d/0x8a0
[ 6384.469596] ? __ceph_caps_issued_mask+0xe7/0x280
[ 6384.469599] ? find_held_lock+0xc9/0xf0
[ 6384.469604] ? __ceph_do_getattr+0x19f/0x290
[ 6384.469608] ? down_read_non_owner+0x1c0/0x1c0
[ 6384.469612] ? do_raw_spin_unlock+0xa3/0x130
[ 6384.469617] ? _raw_spin_unlock+0x24/0x30
[ 6384.469622] ? __lock_acquire.isra.0+0x486/0x770
[ 6384.469629] ? path_openat+0x7ef/0xfe0
[ 6384.469635] ? down_write+0x11e/0x130
[ 6384.469638] down_write+0x11e/0x130
[ 6384.469642] ? down_read_killable+0x1e0/0x1e0
[ 6384.469646] ? __sb_start_write+0x11c/0x170
[ 6384.469650] ? __mnt_want_write+0xb4/0xd0
[ 6384.469655] path_openat+0x7ef/0xfe0
[ 6384.469661] ? path_mountpoint+0x4d0/0x4d0
[ 6384.469667] ? __is_insn_slot_addr+0x93/0xb0
[ 6384.469671] ? kernel_text_address+0x113/0x120
[ 6384.469674] ? __kernel_text_address+0xe/0x30
[ 6384.469679] ? unwind_get_return_address+0x2f/0x50
[ 6384.469683] ? swiotlb_map.cold+0x25/0x25
[ 6384.469687] ? arch_stack_walk+0x8f/0xe0
[ 6384.469692] do_filp_open+0x12b/0x1c0
[ 6384.469695] ? may_open_dev+0x50/0x50
[ 6384.469702] ? __alloc_fd+0x115/0x280
[ 6384.469705] ? lock_downgrade+0x350/0x350
[ 6384.469709] ? do_raw_spin_lock+0x113/0x1d0
[ 6384.469713] ? rwlock_bug.part.0+0x60/0x60
[ 6384.469718] ? do_raw_spin_unlock+0xa3/0x130
[ 6384.469722] ? _raw_spin_unlock+0x24/0x30
[ 6384.469725] ? __alloc_fd+0x115/0x280
[ 6384.469731] do_sys_open+0x1f0/0x2d0
[ 6384.469735] ? filp_open+0x50/0x50
[ 6384.469738] ? switch_fpu_return+0x13e/0x230
[ 6384.469742] ? __do_page_fault+0x4b5/0x670
[ 6384.469748] do_syscall_64+0x63/0x1c0
[ 6384.469753] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 6384.469756] RIP: 0033:0x7fe961434528
[ 6384.469760] Code: 00 00 41 00 3d 00 00 41 00 74 47 48 8d 05 20 4d 0d 00 8b
00 85 c0 75 6b 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00
f0 ff ff 0f 87 94 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
[ 6384.469762] RSP: 002b:7ffd9bbabb20 EFLAGS: 0246 ORIG_RAX:
0101
[ 6384.469765] RAX: ffda RBX: 0242 RCX: 7fe961434528
[ 6384.469767] RDX: 0242 RSI: 7ffd9bbae2a5 RDI: ff9c
[ 6384.469769] RBP: 7ffd9bbae2a5 R08: 0001 R09:
[ 6384.469771] R10: 0180 R11: 0246 R12: 0242
[ 6384.469773] R13: 7ffd9bbabe00 R14: 0180 R15: 0060
[ 6384.470018] Allocated by task 16593:
[ 6384.470562] __kasan_kmalloc.part.0+0x3c/0xa0
[ 6384.470565] kmem_cache_alloc+0xdc/0x240
[ 6384.470569] copy_process+0x1dce/0x27b0
[ 6384.470572] _do_fork+0xec/0x540
[ 6384.470576] __se_sys_clone+0xb2/0x100
[ 6384.470581] do_syscall_64+0x63/0x1c0
[ 6384.470586] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 6384.470823] Freed by task 9:
[ 6384.471235] __kasan_slab_free+0x147/0x200
[ 6384.471240] kmem_cache_free+0x111/0x330
[ 6384.471246] rcu_core+0x2f9/0x830
[ 6384.471251] __do_softirq+0x154/0x486
[ 6384.471493] The buggy address belongs to the object at 8881d5dc9440
which
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On 6/4/19 4:52 AM, Peter Zijlstra wrote:
> On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
>> +static inline struct task_struct *rwsem_read_owner(struct rw_semaphore *sem)
>> +{
>> +return (struct task_struct *)(atomic_long_read(>owner) &
>> +~RWSEM_OWNER_FLAGS_MASK);
>> +}
>> +
>> +/*
>> + * Return the real task structure pointer of the owner and the embedded
>> + * flags in the owner. pflags must be non-NULL.
>> + */
>> +static inline struct task_struct *
>> +rwsem_read_owner_flags(struct rw_semaphore *sem, long *pflags)
>> +{
>> +long owner = atomic_long_read(>owner);
>> +
>> +*pflags = owner & RWSEM_OWNER_FLAGS_MASK;
>> +return (struct task_struct *)(owner & ~RWSEM_OWNER_FLAGS_MASK);
>> +}
> I got confused by the 'read' part in those nanes, I initially thought
> they paired with rwsem_set_reader_owned().
Sorry for the confusion. I initially use "get", but I am afraid that it
may get confused with "get/put" for reference counting. So I used read
instead.
> So I've done 's/rwsem_read_owner/rwsem_owner/g on it.
I am fine with getting rid of the "read" from the function names.
Cheers,
Longman
Re: [PATCH v8 13/19] locking/rwsem: Make rwsem->owner an atomic_long_t
On Mon, May 20, 2019 at 04:59:12PM -0400, Waiman Long wrote:
> +static inline struct task_struct *rwsem_read_owner(struct rw_semaphore *sem)
> +{
> + return (struct task_struct *)(atomic_long_read(>owner) &
> + ~RWSEM_OWNER_FLAGS_MASK);
> +}
> +
> +/*
> + * Return the real task structure pointer of the owner and the embedded
> + * flags in the owner. pflags must be non-NULL.
> + */
> +static inline struct task_struct *
> +rwsem_read_owner_flags(struct rw_semaphore *sem, long *pflags)
> +{
> + long owner = atomic_long_read(>owner);
> +
> + *pflags = owner & RWSEM_OWNER_FLAGS_MASK;
> + return (struct task_struct *)(owner & ~RWSEM_OWNER_FLAGS_MASK);
> +}
I got confused by the 'read' part in those nanes, I initially thought
they paired with rwsem_set_reader_owned().
So I've done 's/rwsem_read_owner/rwsem_owner/g on it.
--- a/kernel/locking/rwsem.c
+++ b/kernel/locking/rwsem.c
@@ -194,10 +194,10 @@ static inline void rwsem_clear_reader_ow
/*
* Return just the real task structure pointer of the owner
*/
-static inline struct task_struct *rwsem_read_owner(struct rw_semaphore *sem)
+static inline struct task_struct *rwsem_owner(struct rw_semaphore *sem)
{
- return (struct task_struct *)(atomic_long_read(>owner) &
- ~RWSEM_OWNER_FLAGS_MASK);
+ return (struct task_struct *)
+ (atomic_long_read(>owner) & ~RWSEM_OWNER_FLAGS_MASK);
}
/*
@@ -205,7 +205,7 @@ static inline struct task_struct *rwsem_
* flags in the owner. pflags must be non-NULL.
*/
static inline struct task_struct *
-rwsem_read_owner_flags(struct rw_semaphore *sem, long *pflags)
+rwsem_owner_flags(struct rw_semaphore *sem, long *pflags)
{
long owner = atomic_long_read(>owner);
@@ -561,7 +561,7 @@ static inline bool rwsem_can_spin_on_own
preempt_disable();
rcu_read_lock();
- owner = rwsem_read_owner_flags(sem, );
+ owner = rwsem_owner_flags(sem, );
if ((flags & RWSEM_NONSPINNABLE) || (owner && !owner_on_cpu(owner)))
ret = false;
rcu_read_unlock();
@@ -590,8 +590,8 @@ enum owner_state {
};
#define OWNER_SPINNABLE(OWNER_NULL | OWNER_WRITER)
-static inline enum owner_state rwsem_owner_state(struct task_struct *owner,
-long flags)
+static inline enum owner_state
+rwsem_owner_state(struct task_struct *owner, long flags)
{
if (flags & RWSEM_NONSPINNABLE)
return OWNER_NONSPINNABLE;
@@ -608,7 +608,7 @@ static noinline enum owner_state rwsem_s
long flags, new_flags;
enum owner_state state;
- owner = rwsem_read_owner_flags(sem, );
+ owner = rwsem_owner_flags(sem, );
state = rwsem_owner_state(owner, flags);
if (state != OWNER_WRITER)
return state;
@@ -620,7 +620,7 @@ static noinline enum owner_state rwsem_s
break;
}
- new = rwsem_read_owner_flags(sem, _flags);
+ new = rwsem_owner_flags(sem, _flags);
if ((new != owner) || (new_flags != flags)) {
state = rwsem_owner_state(new, new_flags);
break;
@@ -1139,7 +1139,7 @@ static inline void __up_write(struct rw_
* sem->owner may differ from current if the ownership is transferred
* to an anonymous writer by setting the RWSEM_NONSPINNABLE bits.
*/
- DEBUG_RWSEMS_WARN_ON((rwsem_read_owner(sem) != current) &&
+ DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) &&
!rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem);
rwsem_clear_owner(sem);
tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, >count);
@@ -1161,7 +1161,7 @@ static inline void __downgrade_write(str
* read-locked region is ok to be re-ordered into the
* write side. As such, rely on RELEASE semantics.
*/
- DEBUG_RWSEMS_WARN_ON(rwsem_read_owner(sem) != current, sem);
+ DEBUG_RWSEMS_WARN_ON(rwsem_owner(sem) != current, sem);
tmp = atomic_long_fetch_add_release(
-RWSEM_WRITER_LOCKED+RWSEM_READER_BIAS, >count);
rwsem_set_reader_owned(sem);
