linux-kernel - Re: [BUG] trigger BUG_ON in mas_store

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-next>] [day] [month] [year] [list]

Message-ID: <20230614155826.cxcpv33hs763gyrg@revolver>
Date:   Wed, 14 Jun 2023 11:58:26 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     John Hsu (許永翰) <John.Hsu@...iatek.com>
Cc:     Andrew Yang (楊智強) 
        <Andrew.Yang@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Qun-wei Lin (林群崴) 
        <Qun-wei.Lin@...iatek.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Chinwen Chang (張錦文) 
        <chinwen.chang@...iatek.com>,
        Casper Li (李中榮) <casper.li@...iatek.com>,
        Kuan-Ying Lee (李冠穎) 
        <Kuan-Ying.Lee@...iatek.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "yuzhao@...gle.com" <yuzhao@...gle.com>,
        "maple-tree@...ts.infradead.org" <maple-tree@...ts.infradead.org>
Subject: Re: [BUG] trigger BUG_ON in mas_store_prealloc when low memory

* John Hsu (許永翰) <John.Hsu@...iatek.com> [230614 03:06]:
> Hi Liam, thanks for your reply.

Sorry, your email response with top posting is hard to follow so I will
do my best to answer your questions.

> 
> 
> 
> version 6.1 or 6.1.x?  Which exact version (git id or version number)
> 
> Our environment is kernel-6.1.25-mainline-android14-5-gdea04bf2c398d.

Okay, I can have a look at 6.1.25 then.

> 
> 
> This BUG_ON() is necessary since this function should _never_ run out of
> 
> memory; this function does not return an error code. mas_preallocate()
> 
> should have gotten you the memory necessary (or returned an -ENOMEM)
> 
> prior to the call to mas_store_prealloc(), so this is probably an
> 
> internal tree problem.
> 
> There is a tree operation being performed here.  mprotect is merging a
> 
> vma by the looks of the call stack.  Why do you think no tree operation
> 
> is necessary?
> 
> As you mentioned, mas_preallocate() should allocate enough node, but there is such functions mas_node_count() in mas_store_prealloc().
> In mas_node_count() checks whether the *mas* has enough nodes, and allocate memory for node if there was no enough nodes in mas.

Right, we call mas_node_count() so that both code paths are used for
preallocations and regular mas_store()/mas_store_gfp().  It shouldn't
take a significant amount of time to verify there is enough nodes.

> I think that if mas_preallocate() allocate enough node, why we check the node count and allocate nodes if there was no enough nodes in mas in mas_node_count()?

We check for the above reason.

> 
> We have seen that there may be some maple_tree operations in merge_vma...

If merge_vma() does anything, then there was an operation to the maple
tree.

> 
> Moreover, would maple_tree provides an API for assigning user's gfp flag for allocating node?

mas_preallocate() and mas_store_gfp() has gfp flags as an argument.  In
your call stack, it will be called in __vma_adjust() as such:

if (mas_preallocate(&mas, vma, GFP_KERNEL))
	return -ENOMEM;

line 715 in v6.1.25

> In rb_tree, we allocate vma_area_struct (rb_node is in this struct.) with GFP_KERNEL, and maple_tree allocate node with GFP_NOWAIT and __GFP_NOWARN.

We use GFP_KERNEL as I explained above for the VMA tree.

It also will drop the lock and retry with GFP_KERNEL on failure
when not using the external lock.  The mmap_lock is configured as an
external lock.

> Allocation will not wait for reclaiming and compacting when there is no enough available memory.
> Is there any concern for this design?

This has been addressed above, but let me know if I missed anything
here.

> 
> 
> I see this is arm64.  Do you have a reproducer?  If you don't have a
> 
> reproducer, I can try stress-ng on amr64 to simulate your workload using
> 
> mprotect, but I need to know the exact kernel version as this issue may
> 
> have been fixed in a later stable release.
> 
> It is offen occur under low memory condiction. Maybe you can try stress-ng on arm64 under high memory stress(e.g. reserved lots of memory).

Okay, I will try arm64 with v6.1.25.

...
> > following are the backtrace:
> 
> > mas_store_prealloc+0x23c/0x484
> 
> > vma_mas_store+0xe4/0x2d0
> 
> > __vma_adjust+0xab0/0x1470
> 
> > vma_merge+0x5b8/0x5d4
> 
> > mprotect_fixup+0x1f4/0x478
> 
> > __arm64_sys_mprotect+0x6b0/0x8f0
> 
> > invoke_syscall+0x84/0x264
> 
> > el0_svc_common+0x118/0x1f0
> 
> > do_el0_svc+0x5c/0x184
> 
> > el0_svc+0x38/0x98
> 

Thanks,
Liam