From patchwork Tue May 2 16:08:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roman Gushchin X-Patchwork-Id: 13229123 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D50D7C77B73 for ; Tue, 2 May 2023 16:09:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 34B51280003; Tue, 2 May 2023 12:09:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F9E9280002; Tue, 2 May 2023 12:09:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E978280003; Tue, 2 May 2023 12:09:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from out-53.mta0.migadu.com (out-53.mta0.migadu.com [91.218.175.53]) by kanga.kvack.org (Postfix) with ESMTP id EF6ED280002 for ; Tue, 2 May 2023 12:09:28 -0400 (EDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1683043768; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u/o5KjzCfkDqLZEgd6My+xebtVjdTGEDkH8rtVpTuiI=; b=IOW3TxD7mQFM3cSirOEqkWfTreKFOytB5wnvSIWgdIE5LdcHx6/C8iFldrSkpepWn3pO1X GW0Qhg/8UOxiky1J59iFD0QN/l1XYb5fVTpqZ8JVJbinwCWAbcp7ivrh67jqD7pe/l0XPO KUjtx9uNRHcRn+hiCbrfChx5FgUlHdo= From: Roman Gushchin To: linux-mm@kvack.org, Andrew Morton Cc: Johannes Weiner , Michal Hocko , Shakeel Butt , Muchun Song , linux-kernel@vger.kernel.org, Roman Gushchin , Dmitry Vyukov , Yosry Ahmed Subject: [PATCH v2 2/2] mm: memcg: use READ_ONCE()/WRITE_ONCE() to access stock->cached Date: Tue, 2 May 2023 09:08:39 -0700 Message-Id: <20230502160839.361544-2-roman.gushchin@linux.dev> In-Reply-To: <20230502160839.361544-1-roman.gushchin@linux.dev> References: <20230502160839.361544-1-roman.gushchin@linux.dev> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: A memcg pointer in the percpu stock can be accessed by drain_all_stock() from another cpu in a lockless way. In theory it might lead to an issue, similar to the one which has been discovered with stock->cached_objcg, where the pointer was zeroed between the check for being NULL and dereferencing. In this case the issue is unlikely a real problem, but to make it bulletproof and similar to stock->cached_objcg, let's annotate all accesses to stock->cached with READ_ONCE()/WTRITE_ONCE(). Signed-off-by: Roman Gushchin Cc: Dmitry Vyukov Cc: Yosry Ahmed Cc: Shakeel Butt Acked-by: Shakeel Butt --- mm/memcontrol.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index c823c35c2ed4..1e364ad495a3 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2275,7 +2275,7 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) local_lock_irqsave(&memcg_stock.stock_lock, flags); stock = this_cpu_ptr(&memcg_stock); - if (memcg == stock->cached && stock->nr_pages >= nr_pages) { + if (memcg == READ_ONCE(stock->cached) && stock->nr_pages >= nr_pages) { stock->nr_pages -= nr_pages; ret = true; } @@ -2290,7 +2290,7 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) */ static void drain_stock(struct memcg_stock_pcp *stock) { - struct mem_cgroup *old = stock->cached; + struct mem_cgroup *old = READ_ONCE(stock->cached); if (!old) return; @@ -2303,7 +2303,7 @@ static void drain_stock(struct memcg_stock_pcp *stock) } css_put(&old->css); - stock->cached = NULL; + WRITE_ONCE(stock->cached, NULL); } static void drain_local_stock(struct work_struct *dummy) @@ -2338,10 +2338,10 @@ static void __refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) struct memcg_stock_pcp *stock; stock = this_cpu_ptr(&memcg_stock); - if (stock->cached != memcg) { /* reset if necessary */ + if (READ_ONCE(stock->cached) != memcg) { /* reset if necessary */ drain_stock(stock); css_get(&memcg->css); - stock->cached = memcg; + WRITE_ONCE(stock->cached, memcg); } stock->nr_pages += nr_pages; @@ -2383,7 +2383,7 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) bool flush = false; rcu_read_lock(); - memcg = stock->cached; + memcg = READ_ONCE(stock->cached); if (memcg && stock->nr_pages && mem_cgroup_is_descendant(memcg, root_memcg)) flush = true;