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 CFBB4C433EF for ; Tue, 15 Mar 2022 15:27:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA9148D0002; Tue, 15 Mar 2022 11:27:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D57468D0001; Tue, 15 Mar 2022 11:27:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C1FF78D0002; Tue, 15 Mar 2022 11:27:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0149.hostedemail.com [216.40.44.149]) by kanga.kvack.org (Postfix) with ESMTP id AE4858D0001 for ; Tue, 15 Mar 2022 11:27:36 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4D2298249980 for ; Tue, 15 Mar 2022 15:27:36 +0000 (UTC) X-FDA: 79247000112.23.3CDC10D Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf28.hostedemail.com (Postfix) with ESMTP id AA13CC0004 for ; Tue, 15 Mar 2022 15:27:35 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 5BAEC1F397; Tue, 15 Mar 2022 15:27:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1647358054; 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=bGKIFDA+teOKkrtKp4Xkol4kV67X0eZCC5YsrJbOYXE=; b=FLzXPnahQQRNoRgqVRipDoxwZzkFUF+LmeTEji/nuoW0TMtofZpWLtiWMYrBYRhni3kvNE 5n9Y6GNVovBsjUCmVHwpR3jkYcvYzAjOSfnaZVLae88qr+5LXI4ecgdrhN8mcRDnZSjIR3 VdYOKtIDP+ZwkBYfS12j4W778NR71Zo= 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 0BB06A3B87; Tue, 15 Mar 2022 15:27:33 +0000 (UTC) Date: Tue, 15 Mar 2022 16:27:30 +0100 From: Michal Hocko To: Miaohe Lin Cc: akpm@linux-foundation.org, kosaki.motohiro@jp.fujitsu.com, mgorman@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/mempolicy: fix potential mpol_new leak in shared_policy_replace Message-ID: References: <20220311093624.39546-1-linmiaohe@huawei.com> <26577566-ae1e-801c-8c64-89c2c89a487d@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <26577566-ae1e-801c-8c64-89c2c89a487d@huawei.com> X-Rspam-User: X-Rspamd-Queue-Id: AA13CC0004 X-Stat-Signature: okkh19wumenxxgturqosnen36sxjhq8g Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=FLzXPnah; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com X-Rspamd-Server: rspam03 X-HE-Tag: 1647358055-302804 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 Tue 15-03-22 21:42:29, Miaohe Lin wrote: > On 2022/3/15 0:44, Michal Hocko wrote: > > On Fri 11-03-22 17:36:24, Miaohe Lin wrote: > >> If mpol_new is allocated but not used in restart loop, mpol_new will be > >> freed via mpol_put before returning to the caller. But refcnt is not > >> initialized yet, so mpol_put could not do the right things and might > >> leak the unused mpol_new. > > > > The code is really hideous but is there really any bug there? AFAICS the > > new policy is only allocated in if (n->end > end) branch and that one > > will set the reference count on the retry. Or am I missing something? > > > > Many thanks for your comment. > IIUC, new policy is allocated via the below code: > > shared_policy_replace: > alloc_new: > write_unlock(&sp->lock); > ret = -ENOMEM; > n_new = kmem_cache_alloc(sn_cache, GFP_KERNEL); > if (!n_new) > goto err_out; > mpol_new = kmem_cache_alloc(policy_cache, GFP_KERNEL); > if (!mpol_new) > goto err_out; > goto restart; > > And mpol_new' reference count will be set before used in n->end > end case. But > if that is "not" the case, i.e. mpol_new is not inserted into the rb_tree, mpol_new > will be freed via mpol_put before return: One thing I have missed previously is that the lock is dropped during the allocation so I guess the memory policy could have been changed during that time. Is this possible? Have you explored this possibility? Is this a theoretical problem or it can be triggered intentionally. These details would be really interesting for the changelog so that we can judge how important this would be. -- Michal Hocko SUSE Labs