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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D2DD6C48BC3 for ; Wed, 21 Feb 2024 08:38:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 67CA26B0072; Wed, 21 Feb 2024 03:38:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 62FA56B0074; Wed, 21 Feb 2024 03:38:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4A6AE6B0075; Wed, 21 Feb 2024 03:38:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 36F6D6B0072 for ; Wed, 21 Feb 2024 03:38:30 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C9A6F140980 for ; Wed, 21 Feb 2024 08:38:29 +0000 (UTC) X-FDA: 81815159538.18.FD03749 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf14.hostedemail.com (Postfix) with ESMTP id DA418100008 for ; Wed, 21 Feb 2024 08:38:26 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Q5SGrCmB; dkim=pass header.d=suse.com header.s=susede1 header.b=Q5SGrCmB; spf=pass (imf14.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708504707; a=rsa-sha256; cv=none; b=jGG4oV5WmjFYu6sZG6KT3pjq1AlZFl4HYIsYspRE0baD2J9YuOXtYWqt3ylD27s1eAUTeU C54xgNJS2v3HQImmobzMDTvrMHsSJPP8kUht2s2q6RVozTf5junluFONQBNAeIYo5F+/x8 rFuB7Dsr6NXBYWQmmuMLfNsnSC001y8= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Q5SGrCmB; dkim=pass header.d=suse.com header.s=susede1 header.b=Q5SGrCmB; spf=pass (imf14.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708504707; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=OELScSOCYwRfM6hUA5F7SiQmd9fiPNXBAjELCHScvzA=; b=T7TH0WDZ5S61o06vUHsmAdJEFLyfdxHQyToryxmGO6I8XveXT9iUWpNqUWTPL5+O3HFUy2 uK0a1vRaEzvcF6K26yN+TUAuxDtNRpCKqiGqwqBkpdXw7TGbbi3+PKVac6osXXHQBwG4F8 Rnj9gqpp6cBncD9KcQT5kRfFKUEqHTo= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 0176D22256; Wed, 21 Feb 2024 08:38:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1708504705; 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=OELScSOCYwRfM6hUA5F7SiQmd9fiPNXBAjELCHScvzA=; b=Q5SGrCmBdBOY3fDfhVeXTTJUjsMAFhQNhubHvqPNFfgp1lbzabuVbNOQbWgpQUTpt4ZfuG DS7J7tumeezkrVOlJSNLi5iwmG5TxMGAcaL7FZnHGi0Qll4sgFOpvnf3c/5XfyGETbw2FP 0JaPHNcfjyVjvKUosb5CVl2buDux7rU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1708504705; 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=OELScSOCYwRfM6hUA5F7SiQmd9fiPNXBAjELCHScvzA=; b=Q5SGrCmBdBOY3fDfhVeXTTJUjsMAFhQNhubHvqPNFfgp1lbzabuVbNOQbWgpQUTpt4ZfuG DS7J7tumeezkrVOlJSNLi5iwmG5TxMGAcaL7FZnHGi0Qll4sgFOpvnf3c/5XfyGETbw2FP 0JaPHNcfjyVjvKUosb5CVl2buDux7rU= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id D4DB9139D0; Wed, 21 Feb 2024 08:38:24 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id w3XwMIC21WVpKQAAD6G6ig (envelope-from ); Wed, 21 Feb 2024 08:38:24 +0000 Date: Wed, 21 Feb 2024 09:38:24 +0100 From: Michal Hocko To: "GONG, Ruiqi" Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , cgroups@vger.kernel.org, linux-mm@kvack.org, Wang Weiyang , Xiu Jianfeng Subject: Re: [PATCH stable] memcg: add refcnt for pcpu stock to avoid UAF problem in drain_all_stock() Message-ID: References: <20240221081801.69764-1-gongruiqi1@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240221081801.69764-1-gongruiqi1@huawei.com> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: DA418100008 X-Stat-Signature: q3u6ui9i6wwktku4cxsz1rdkfny5ooe3 X-Rspam-User: X-HE-Tag: 1708504706-218697 X-HE-Meta: U2FsdGVkX1+TycNtckXPSCBCglJ5kzOLm2slY9p0QmM+C+pDE7KEA+SCscDdMrGdtbvmShYbXvHLdez5mfKnLKdEMQ9LrlFq82GpjKu10cPA6/jk1gYdKkE1RAL11RC0LW/d4+Z0MPQC+GI3XHN898Gu5bpiGfFUFR0Z076UtTyAGjaKOh5IaZuDGA8Z0ZyCISWXMUWOmjgtPoGBlwjF66aOh54u6jmahQXorEhv+kP5mAS4ykVzDuYWLrFs52tIzipVSBiRXMMJURxgp9F7i3sAOL8+zHwgif0Ur3LFoGaPVVs0r2cmDrJH8MRIH0KJP5LnXiSK4l57pe3mnLJ9AQFaPHi59E+M/ULJG7Uhye3M8/gTdp5hNxwX8WFfXtr7K9NpXVm5k9gN5lCgZQGIorBqxTcxvciqPzM4Apb0qx2qvhA4QMezNauzFeooTIeFpzg6OJJaGrTwNUMU37a+6stcQiRXM6XesnFA5anIzSDUvKFCAlkW8fwLgrx6oXuuO2LNRykrEsa7Q+zzr8Zmu9gOLMpeTxi7CwMCk1Kg1vDkkgFc0WhYBpd79cK6GjauTCKSPzxkEym9sj8n0dsQbZCeyC1jM9FdJoz3A1l5TK7YPjhbSQSy24TFfnijyc14vEplfjGbPYYzg6wU1B/CyfT5I1SoJntjPGGCwIXWTkKXBYiayiP8/BxHT0eQWcgFVo3e0JuUJmxHn01RMVwVv4V9fmBjE5EHVtvfYaFVJPic2IBX4Y+E8kvCvddaHbgWNhJGPRld/V39/FGJVqg0t9xPye2PuRAhJMUQQQy8Em33wV4/8XpXV7rbUnU4WbfrP+SyzG2eAI11BcX++F7cxfwQkEZxUeCBTutksiaEaB+i7IxS4nUqudHjD8zWqJLI90FQ6I0sK6jOI8IwnIembzcIjiaoF+XY2aEPGUn9BkGTso0mrEDMMfaT+2QvfzAb 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: List-Subscribe: List-Unsubscribe: On Wed 21-02-24 16:18:01, GONG, Ruiqi wrote: > commit 1a3e1f40962c445b997151a542314f3c6097f8c3 upstream. I think it would be good to mention that this is only a partial backport and also explain why to do a partial rather than the full one. > There was a kernel panic happened on an in-house environment running > 3.10, and the same problem was reproduced on 4.19: > > general protection fault: 0000 [#1] SMP PTI > CPU: 1 PID: 2085 Comm: bash Kdump: loaded Tainted: G L 4.19.90+ #7 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 > RIP: 0010 drain_all_stock+0xad/0x140 > Code: 00 00 4d 85 ff 74 2c 45 85 c9 74 27 4d 39 fc 74 42 41 80 bc 24 28 04 00 00 00 74 17 49 8b 04 24 49 8b 17 48 8b 88 90 02 00 00 <48> 39 8a 90 02 00 00 74 02 eb 86 48 63 88 3c 01 00 00 39 8a 3c 01 > RSP: 0018:ffffa7efc5813d70 EFLAGS: 00010202 > RAX: ffff8cb185548800 RBX: ffff8cb89f420160 RCX: ffff8cb1867b6000 > RDX: babababababababa RSI: 0000000000000001 RDI: 0000000000231876 > RBP: 0000000000000000 R08: 0000000000000415 R09: 0000000000000002 > R10: 0000000000000000 R11: 0000000000000001 R12: ffff8cb186f89040 > R13: 0000000000020160 R14: 0000000000000001 R15: ffff8cb186b27040 > FS: 00007f4a308d3740(0000) GS:ffff8cb89f440000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007ffe4d634a68 CR3: 000000010b022000 CR4: 00000000000006e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > mem_cgroup_force_empty_write+0x31/0xb0 > cgroup_file_write+0x60/0x140 > ? __check_object_size+0x136/0x147 > kernfs_fop_write+0x10e/0x190 > __vfs_write+0x37/0x1b0 > ? selinux_file_permission+0xe8/0x130 > ? security_file_permission+0x2e/0xb0 > vfs_write+0xb6/0x1a0 > ksys_write+0x57/0xd0 > do_syscall_64+0x63/0x250 > ? async_page_fault+0x8/0x30 > entry_SYSCALL_64_after_hwframe+0x5c/0xc1 > Modules linked in: ... > > It is found that in case of stock->nr_pages == 0, the memcg on > stock->cached could be freed due to its refcnt decreased to 0, which > made stock->cached become a dangling pointer. It could cause a UAF > problem in drain_all_stock() in the following concurrent scenario. Note > that drain_all_stock() doesn't disable irq but only preemption. > > CPU1 CPU2 > ============================================================================== > stock->cached = memcgA (freed) > drain_all_stock(memcgB) > rcu_read_lock() > memcg = CPU1's stock->cached (memcgA) > (interrupted) > refill_stock(memcgC) > drain_stock(memcgA) > stock->cached = memcgC > stock->nr_pages += xxx (> 0) > stock->nr_pages > 0 > mem_cgroup_is_descendant(memcgA, memcgB) [UAF] > rcu_read_unlock() > > This problem is, unintenionally, fixed at 5.9, where commit 1a3e1f40962c > ("mm: memcontrol: decouple reference counting from page accounting") > adds memcg refcnt for stock. Therefore affected LTS versions include > 4.19 and 5.4. > > For 4.19, memcg's css offline process doesn't call drain_all_stock(). so > it's easier for the released memcg to be left on the stock. For 5.4, > although mem_cgroup_css_offline() does call drain_all_stock(), but the > flushing could be skipped when stock->nr_pages happens to be 0, and > besides the async draining could be delayed and take place after the UAF > problem has happened. > > Fix this problem by adding (and decreasing) memcg's refcnt when memcg is > put onto (and removed from) stock, just like how commit 1a3e1f40962c > ("mm: memcontrol: decouple reference counting from page accounting") > does. After all, "being on the stock" is a kind of reference with > regards to memcg. As such, it's guaranteed that a css on stock would not > be freed. What does prevent from the following? refill_stock(memcgC) drain_all_stock(memcgB) drain_stock(memcgA) rcu_read_lock() css_put(old->css) memcgA = stock->cached mem_cgroup_is_descendant(memcgA, memcgB) UAF stock->cached = NULL > > Cc: stable@vger.kernel.org # 4.19 5.4 > Fixes: cdec2e4265df ("memcg: coalesce charging via percpu storage") > Signed-off-by: GONG, Ruiqi > --- > mm/memcontrol.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5a366cf79821..8c04296df1c7 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2015,6 +2015,9 @@ static void drain_stock(struct memcg_stock_pcp *stock) > { > struct mem_cgroup *old = stock->cached; > > + if (!old) > + return; > + > if (stock->nr_pages) { > page_counter_uncharge(&old->memory, stock->nr_pages); > if (do_memsw_account()) > @@ -2022,6 +2025,8 @@ static void drain_stock(struct memcg_stock_pcp *stock) > css_put_many(&old->css, stock->nr_pages); > stock->nr_pages = 0; > } > + > + css_put(&old->css); > stock->cached = NULL; > } > > @@ -2057,6 +2062,7 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > stock = this_cpu_ptr(&memcg_stock); > if (stock->cached != memcg) { /* reset if necessary */ > drain_stock(stock); > + css_get(&memcg->css); > stock->cached = memcg; > } > stock->nr_pages += nr_pages; > -- > 2.25.1 > -- Michal Hocko SUSE Labs