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 3FF9EC433FE for ; Mon, 3 Oct 2022 13:32:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 56BAB6B0072; Mon, 3 Oct 2022 09:32:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 519F26B0073; Mon, 3 Oct 2022 09:32:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E14E8E0001; Mon, 3 Oct 2022 09:32:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 2BE046B0072 for ; Mon, 3 Oct 2022 09:32:28 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B2AC9805B7 for ; Mon, 3 Oct 2022 13:32:27 +0000 (UTC) X-FDA: 79979727534.20.32977C9 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf17.hostedemail.com (Postfix) with ESMTP id 1ECFA40005 for ; Mon, 3 Oct 2022 13:32:26 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 9A00D1F93B; Mon, 3 Oct 2022 13:32:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1664803945; 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=SXYAxH1bjpXJNmDeNrO3jdecyDEt6cOF5K7wMtb61kM=; b=AvbdyVLxq5Z+EcCLLFioU+L7qcQqOl6LtbzQtgHMAeZUNT1RktaGG38nHYaoIIgTqMnKxB Svp4jswK4lj9pjzH1Xf0AHptLGE5E+UGrB534Y8XlDAUoWJX+X8X84YJiCsjdx7ad4Ylvi 2ozg5D51XZN1MRCwfFcDUk6sw0JV618= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 7BBD11332F; Mon, 3 Oct 2022 13:32:25 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 1B7dG2nkOmNzVAAAMHmgww (envelope-from ); Mon, 03 Oct 2022 13:32:25 +0000 Date: Mon, 3 Oct 2022 15:32:24 +0200 From: Michal Hocko To: Alexander Fedorov Cc: Roman Gushchin , Johannes Weiner , Shakeel Butt , Vladimir Davydov , Muchun Song , Sebastian Andrzej Siewior , cgroups@vger.kernel.org, linux-mm@kvack.org Subject: Re: Possible race in obj_stock_flush_required() vs drain_obj_stock() Message-ID: References: <1664546131660.1777662787.1655319815@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664803947; 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=SXYAxH1bjpXJNmDeNrO3jdecyDEt6cOF5K7wMtb61kM=; b=6bHUeGcFN85tRzSJz7QZ+xr0I59Fc65d5nwTVOxVdzOOWOiE/hMjbXbEB95/lqXGga7lqP mLaPUvCvx7s9OvOb76Hd8FjxCHvvhtrzngzOCI53F1G62+ptDmVcbpcaxfDrfN9l5gTFNH e/gBCNdbNmM6gwposjwr3AEW0ldi4/U= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=AvbdyVLx; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf17.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664803947; a=rsa-sha256; cv=none; b=2rfoAuBfmp3yYirY71WqjhvlK45+p/IwnfVPq0aCvModDfYoxInexO1fhsy48ivy+nMJ8/ EWf4kV2y+b1GzdsbiI5+pwpFIisTPpoH+YVtFnsHahFpQFDQ8/yhoFbRvATk9fSRH5r9uO KYduZZ8HxOy6OZM1p/RguxKNaxamS+8= X-Rspamd-Queue-Id: 1ECFA40005 Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=AvbdyVLx; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf17.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com X-Rspam-User: X-Rspamd-Server: rspam10 X-Stat-Signature: iuksiafetcw6ceuwrtpmofzrwqkxkk4x X-HE-Tag: 1664803946-554717 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: On Mon 03-10-22 15:47:10, Alexander Fedorov wrote: > On 02.10.2022 19:16, Roman Gushchin wrote: > > On Sat, Oct 01, 2022 at 03:38:43PM +0300, Alexander Fedorov wrote: > >> Tested READ_ONCE() patch and it works. > > > > Thank you! > > > >> But are rcu primitives an overkill? > >> For me they are documenting how actually complex is synchronization here. > > > > I agree, however rcu primitives will add unnecessary barriers on hot paths. > > In this particular case most accesses to stock->cached_objcg are done from > > a local cpu, so no rcu primitives are needed. So in my opinion using a > > READ_ONCE() is preferred. > > Understood, then here is patch that besides READ_ONCE() also fixes mentioned > use-after-free that exists in 5.10. In mainline the drain_obj_stock() part > of the patch should be skipped. > > Should probably be also Signed-off-by: Roman Gushchin > but I am not sure if I have rights to add that :) > > > mm/memcg: fix race in obj_stock_flush_required() vs drain_obj_stock() > > When obj_stock_flush_required() is called from drain_all_stock() it > reads the `memcg_stock->cached_objcg` field twice for another CPU's > per-cpu variable, leading to TOCTTOU race: another CPU can > simultaneously enter drain_obj_stock() and clear its own instance of > `memcg_stock->cached_objcg`. > > Another problem is in drain_obj_stock() which sets `cached_objcg` to > NULL after freeing which might lead to use-after-free. > > To fix it use READ_ONCE() for TOCTTOU problem and also clear the > `cached_objcg` pointer earlier in drain_obj_stock() for use-after-free > problem. > > Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API") > Signed-off-by: Alexander Fedorov > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c1152f8747..56bd5ea6d3 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3197,17 +3197,30 @@ static void drain_obj_stock(struct memcg_stock_pcp *stock) > stock->nr_bytes = 0; > } > > - obj_cgroup_put(old); > + /* > + * Clear pointer before freeing memory so that > + * drain_all_stock() -> obj_stock_flush_required() > + * does not see a freed pointer. > + */ > stock->cached_objcg = NULL; > + obj_cgroup_put(old); Do we need barrier() or something else to ensure there is no reordering? I am not reallyu sure what kind of barriers are implied by the pcp ref counting. -- Michal Hocko SUSE Labs