linux-kernel - Re: [PATCH 4/5] mm, memcg: avoid possible NULL pointer dereferencing in mem_cgroup

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:   Mon, 2 Aug 2021 08:43:13 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     Roman Gushchin <guro@...com>, hannes@...xchg.org,
        vdavydov.dev@...il.com, akpm@...ux-foundation.org,
        shakeelb@...gle.com, willy@...radead.org, alexs@...nel.org,
        richard.weiyang@...il.com, songmuchun@...edance.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org
Subject: Re: [PATCH 4/5] mm, memcg: avoid possible NULL pointer dereferencing
 in mem_cgroup_init()

On Sat 31-07-21 10:05:51, Miaohe Lin wrote:
> On 2021/7/30 14:44, Michal Hocko wrote:
> > On Thu 29-07-21 20:12:43, Roman Gushchin wrote:
> >> On Thu, Jul 29, 2021 at 08:57:54PM +0800, Miaohe Lin wrote:
> >>> rtpn might be NULL in very rare case. We have better to check it before
> >>> dereferencing it. Since memcg can live with NULL rb_tree_per_node in
> >>> soft_limit_tree, warn this case and continue.
> >>>
> >>> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> >>> ---
> >>>  mm/memcontrol.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >>> index 5b4592d1e0f2..70a32174e7c4 100644
> >>> --- a/mm/memcontrol.c
> >>> +++ b/mm/memcontrol.c
> >>> @@ -7109,6 +7109,8 @@ static int __init mem_cgroup_init(void)
> >>>  		rtpn = kzalloc_node(sizeof(*rtpn), GFP_KERNEL,
> >>>  				    node_online(node) ? node : NUMA_NO_NODE);
> >>>  
> >>> +		if (WARN_ON_ONCE(!rtpn))
> >>> +			continue;
> >>
> >> I also really doubt that it makes any sense to continue in this case.
> >> If this allocations fails (at the very beginning of the system's life, it's an __init function),
> >> something is terribly wrong and panic'ing on a NULL-pointer dereference sounds like
> >> a perfect choice.
> > 
> > Moreover this is 24B allocation during early boot. Kernel will OOM and
> > panic when not being able to find any victim. I do not think we need to
> 
> Agree with you. But IMO it may not be a good idea to leave the rtpn without NULL check. We should defend
> it though it could hardly happen. But I'm not insist on this check. I will drop this patch if you insist.

It is not that I would insist. I just do not see any point in the code
churn. This check is not going to ever trigger and there is nothing you
can do to recover anyway so crashing the kernel is likely the only
choice left.
-- 
Michal Hocko
SUSE Labs