From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6CAA1C432BE for ; Mon, 2 Aug 2021 06:43:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 52CE4610A0 for ; Mon, 2 Aug 2021 06:43:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232394AbhHBGn1 (ORCPT ); Mon, 2 Aug 2021 02:43:27 -0400 Received: from smtp-out1.suse.de ([195.135.220.28]:49236 "EHLO smtp-out1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232331AbhHBGnZ (ORCPT ); Mon, 2 Aug 2021 02:43:25 -0400 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id EC3E121FC1; Mon, 2 Aug 2021 06:43:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1627886595; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=kKU12K2fBZbHqr4t5nmrbX+sTG0uq/OYc4mzSKOcY+E=; b=ItZF/JTnMqza/U+as0Qgt/GrVy+8VM1qC0l9HUtskjjbfn77AOblkta5K0FTgJiLUeh9Mw DdqJFqHwU0JniuUu0LJhAsf07/XaWh1xfVTZ5mjIwN2OdEBMxIz4qBTcOBnmJJnObwnS8c 8XQkm1MjNBQBx8WecYSxILo3+yYTO6s= Received: from suse.cz (unknown [10.100.201.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 46A27A3B83; Mon, 2 Aug 2021 06:43:15 +0000 (UTC) Date: Mon, 2 Aug 2021 08:43:13 +0200 From: Michal Hocko To: Miaohe Lin Cc: Roman Gushchin , hannes@cmpxchg.org, vdavydov.dev@gmail.com, akpm@linux-foundation.org, shakeelb@google.com, willy@infradead.org, alexs@kernel.org, richard.weiyang@gmail.com, songmuchun@bytedance.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH 4/5] mm, memcg: avoid possible NULL pointer dereferencing in mem_cgroup_init() Message-ID: References: <20210729125755.16871-1-linmiaohe@huawei.com> <20210729125755.16871-5-linmiaohe@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > >>> --- > >>> 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