* [RFC PATCH] mm/vmalloc: fix vbq->free breakage
@ 2024-05-30 9:31 hailong.liu
2024-05-30 20:03 ` Andrew Morton
2024-05-30 20:05 ` Andrew Morton
0 siblings, 2 replies; 4+ messages in thread
From: hailong.liu @ 2024-05-30 9:31 UTC (permalink / raw)
To: akpm
Cc: urezki, hch, lstoakes, 21cnbao, linux-mm, linux-kernel, xiang,
guangye.yang, zhaoyang.huang, hailong.liu, Hailong . Liu
From: "hailong.liu" <hailong.liu@oppo.com>
The function xa_for_each() in _vm_unmap_aliases() loops through all
vbs. However, since commit 062eacf57ad9 ("mm: vmalloc: remove a global
vmap_blocks xarray") the vb from xarray may not be on the corresponding
CPU vmap_block_queue. Consequently, purge_fragmented_block() might
use the wrong vbq->lock to protect the free list, leading to vbq->free
breakage.
Signed-off-by: Hailong.Liu <liuhailong@oppo.com>
Reported-by: Guangye Yang <guangye.yang@mediatek.com>
---
mm/vmalloc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d12a17fc0c17..869e7788a7d5 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2269,10 +2269,9 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
for_each_possible_cpu(cpu) {
struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, cpu);
struct vmap_block *vb;
- unsigned long idx;
rcu_read_lock();
- xa_for_each(&vbq->vmap_blocks, idx, vb) {
+ list_for_each_entry_rcu(vb, &vbq->free, free_list) {
spin_lock(&vb->lock);
/*
---
https://lore.kernel.org/all/20240530025144.1570865-1-zhaoyang.huang@unisoc.com/
BTW, zhangyang also encounter the same issue, maybe revert commit not a
better solution. we need a map to get vbq from vb.
--
2.30.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmalloc: fix vbq->free breakage
2024-05-30 9:31 [RFC PATCH] mm/vmalloc: fix vbq->free breakage hailong.liu
@ 2024-05-30 20:03 ` Andrew Morton
2024-05-31 2:53 ` Hailong Liu
2024-05-30 20:05 ` Andrew Morton
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2024-05-30 20:03 UTC (permalink / raw)
To: hailong.liu
Cc: urezki, hch, lstoakes, 21cnbao, linux-mm, linux-kernel, xiang,
guangye.yang, zhaoyang.huang, Hailong . Liu
On Thu, 30 May 2024 17:31:08 +0800 <hailong.liu@oppo.com> wrote:
> From: "hailong.liu" <hailong.liu@oppo.com>
>
> The function xa_for_each() in _vm_unmap_aliases() loops through all
> vbs. However, since commit 062eacf57ad9 ("mm: vmalloc: remove a global
> vmap_blocks xarray") the vb from xarray may not be on the corresponding
> CPU vmap_block_queue. Consequently, purge_fragmented_block() might
> use the wrong vbq->lock to protect the free list, leading to vbq->free
> breakage.
What are the userspace-visible runtime effects of this change?
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index d12a17fc0c17..869e7788a7d5 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2269,10 +2269,9 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
> for_each_possible_cpu(cpu) {
> struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, cpu);
> struct vmap_block *vb;
> - unsigned long idx;
>
> rcu_read_lock();
> - xa_for_each(&vbq->vmap_blocks, idx, vb) {
> + list_for_each_entry_rcu(vb, &vbq->free, free_list) {
> spin_lock(&vb->lock);
>
> /*
> ---
> https://lore.kernel.org/all/20240530025144.1570865-1-zhaoyang.huang@unisoc.com/
> BTW, zhangyang also encounter the same issue, maybe revert commit not a
> better solution. we need a map to get vbq from vb.
That patch didn't describe the runtime effects either.
Folks, please always do this. So that others can decide whether their
kernel needs the fix, and so that others can decide whether this fix
might address an issue which their users are reporting.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmalloc: fix vbq->free breakage
2024-05-30 9:31 [RFC PATCH] mm/vmalloc: fix vbq->free breakage hailong.liu
2024-05-30 20:03 ` Andrew Morton
@ 2024-05-30 20:05 ` Andrew Morton
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2024-05-30 20:05 UTC (permalink / raw)
To: hailong.liu
Cc: urezki, hch, lstoakes, 21cnbao, linux-mm, linux-kernel, xiang,
guangye.yang, zhaoyang.huang, Hailong . Liu
On Thu, 30 May 2024 17:31:08 +0800 <hailong.liu@oppo.com> wrote:
> From: "hailong.liu" <hailong.liu@oppo.com>
>
> The function xa_for_each() in _vm_unmap_aliases() loops through all
> vbs. However, since commit 062eacf57ad9 ("mm: vmalloc: remove a global
> vmap_blocks xarray") the vb from xarray may not be on the corresponding
> CPU vmap_block_queue. Consequently, purge_fragmented_block() might
> use the wrong vbq->lock to protect the free list, leading to vbq->free
> breakage.
>
> ...
>
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2269,10 +2269,9 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
> for_each_possible_cpu(cpu) {
> struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, cpu);
> struct vmap_block *vb;
> - unsigned long idx;
>
> rcu_read_lock();
> - xa_for_each(&vbq->vmap_blocks, idx, vb) {
> + list_for_each_entry_rcu(vb, &vbq->free, free_list) {
> spin_lock(&vb->lock);
>
> /*
> ---
> https://lore.kernel.org/all/20240530025144.1570865-1-zhaoyang.huang@unisoc.com/
> BTW, zhangyang also encounter the same issue, maybe revert commit not a
> better solution. we need a map to get vbq from vb.
I borrowed the Fixes: from that patch and added cc:stable, pending
confirmation that the runtime effects are significant.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mm/vmalloc: fix vbq->free breakage
2024-05-30 20:03 ` Andrew Morton
@ 2024-05-31 2:53 ` Hailong Liu
0 siblings, 0 replies; 4+ messages in thread
From: Hailong Liu @ 2024-05-31 2:53 UTC (permalink / raw)
To: Andrew Morton
Cc: urezki, hch, lstoakes, 21cnbao, linux-mm, linux-kernel, xiang,
guangye.yang, zhaoyang.huang, Hailong . Liu
On 5/31/2024 4:03 AM, Andrew Morton wrote:
> On Thu, 30 May 2024 17:31:08 +0800 <hailong.liu@oppo.com> wrote:
>
>> From: "hailong.liu" <hailong.liu@oppo.com>
>>
>> The function xa_for_each() in _vm_unmap_aliases() loops through all
>> vbs. However, since commit 062eacf57ad9 ("mm: vmalloc: remove a global
>> vmap_blocks xarray") the vb from xarray may not be on the corresponding
>> CPU vmap_block_queue. Consequently, purge_fragmented_block() might
>> use the wrong vbq->lock to protect the free list, leading to vbq->free
>> breakage.
>
> What are the userspace-visible runtime effects of this change?
>
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index d12a17fc0c17..869e7788a7d5 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -2269,10 +2269,9 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
>> for_each_possible_cpu(cpu) {
>> struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, cpu);
>> struct vmap_block *vb;
>> - unsigned long idx;
>>
>> rcu_read_lock();
>> - xa_for_each(&vbq->vmap_blocks, idx, vb) {
>> + list_for_each_entry_rcu(vb, &vbq->free, free_list) {
>> spin_lock(&vb->lock);
>>
>> /*
>> ---
>> https://lore.kernel.org/all/20240530025144.1570865-1-zhaoyang.huang@unisoc.com/
>> BTW, zhangyang also encounter the same issue, maybe revert commit not a
>> better solution. we need a map to get vbq from vb.
>
> That patch didn't describe the runtime effects either.
>
> Folks, please always do this. So that others can decide whether their
> kernel needs the fix, and so that others can decide whether this fix
> might address an issue which their users are reporting.
Thanks for suggestion. I send v2 in
https://lore.kernel.org/lkml/20240531024820.5507-1-hailong.liu@oppo.com/T/#u
I promise to make my future commit messages comprehensive, in order to help others.
Brs,
Hailong.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-31 2:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-30 9:31 [RFC PATCH] mm/vmalloc: fix vbq->free breakage hailong.liu
2024-05-30 20:03 ` Andrew Morton
2024-05-31 2:53 ` Hailong Liu
2024-05-30 20:05 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox