linux-kernel - Re: [PATCH v7 1/2] mm/vmalloc: Moved macros with no functional change happened

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 7 Mar 2024 11:01:11 +0800
From: "Huang, Rulin" <rulin.huang@...el.com>
To: Baoquan He <bhe@...hat.com>, Uladzislau Rezki <urezki@...il.com>
Cc: akpm@...ux-foundation.org, colin.king@...el.com, hch@...radead.org,
 linux-kernel@...r.kernel.org, linux-mm@...ck.org, lstoakes@...il.com,
 tianyou.li@...el.com, tim.c.chen@...el.com, wangyang.guo@...el.com,
 zhiguo.zhou@...el.com
Subject: Re: [PATCH v7 1/2] mm/vmalloc: Moved macros with no functional change
 happened

We have made changes based on your latest suggestions.
1.Removed bugs_on.
2.Removed adjustion of macros.

We submitted patch v8 based on this. Thanks to Baoquan for the
discussion, and could you please help to review and confirm if there are
any problems on the latest version?

On 2024/3/7 9:23, Baoquan He wrote:
> On 03/06/24 at 08:01pm, Uladzislau Rezki wrote:
>> On Fri, Mar 01, 2024 at 10:54:16AM -0500, rulinhuang wrote:
> ......
>>
>> Sorry for the late answer, i also just noticed this email. It was not in
>> my inbox...
>>
>> OK, now you move part of the per-cpu allocator on the top and leave
>> another part down making it split. This is just for the:
>>
>> BUG_ON(va_flags & VMAP_RAM);
>>
>> VMAP_RAM macro. Do we really need this BUG_ON()?
> 
> Sorry, I suggested that when reviewing v5:
> https://lore.kernel.org/all/ZdiltpK5fUvwVWtD@MiWiFi-R3L-srv/T/#u
> 
> About part of per-cpu kva allocator moving and the split making, I would
> argue that we will have vmap_nodes defintion and basic helper functions
> like addr_to_node_id() etc at top, and leave other part like
> size_to_va_pool(), node_pool_add_va() etc down. These are similar.
> 
> While about whether we should add 'BUG_ON(va_flags & VMAP_RAM);', I am
> not sure about it. When I suggested that, I am also hesitant. From the
> current code, alloc_vmap_area() is called in below three functions, only
> __get_vm_area_node() will pass the non-NULL vm. 
>  new_vmap_block()     -|
>  vm_map_ram()         ----> alloc_vmap_area()
>  __get_vm_area_node() -|
> 
> It could be wrongly passed in the future? Only checking if vm is
> non-NULL makes me feel a little unsafe. While I am fine if removing the
> BUG_ON, because there's no worry in the current code. We can wait and
> see in the future.
> 
>        if (vm) {
>                BUG_ON(va_flags & VMAP_RAM);
>                setup_vmalloc_vm(vm, va, flags, caller);
>        }
> 
> Thanks
> Baoquan
>