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 1664BC04A68 for ; Thu, 28 Jul 2022 02:37:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 48715940034; Wed, 27 Jul 2022 22:37:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 435C1940012; Wed, 27 Jul 2022 22:37:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2FD30940034; Wed, 27 Jul 2022 22:37:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 2105A940012 for ; Wed, 27 Jul 2022 22:37:40 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E1EBB80FB2 for ; Thu, 28 Jul 2022 02:37:39 +0000 (UTC) X-FDA: 79734947838.02.7C9615B Received: from mail-ua1-f66.google.com (mail-ua1-f66.google.com [209.85.222.66]) by imf23.hostedemail.com (Postfix) with ESMTP id 87A2C140012 for ; Thu, 28 Jul 2022 02:37:38 +0000 (UTC) Received: by mail-ua1-f66.google.com with SMTP id q46so273370uaq.0 for ; Wed, 27 Jul 2022 19:37:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MfJSj7i/OoCG7kZZKpDbdtqWbIKHdQEmX9HjfPFjqLs=; b=fRFnidABVPlzkU1RKqySnsqczmNZ4fJ07KJJoNuzJl/Wdi0H9s02JiQtUyPZ/xyBhh qcw6mqIm8znBQNIxPxmgDJsN0KShkqCJ2+uEiMoZrqZIAo9LxSpzEqnEQUSP3l6kLAPg JmEoO33JfxJ8INlKC7FBXNscD/wrqcQ0qCjCYbsVmlQFdQtVIZeE0aIkUZt55lyUbheS 8sIRsQiDWh4yRRrxjyb8rUY53uQdPt/mv9vvjCvBmAzaUxH/ASdXIb10jWHbBxLlVg5I qHKOL5GV0L88a2VLfoVASih9dBWJBox6WDrMpSlfX9hGlh2VGq+p5tBLkCPWi6E+aoKi ztrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MfJSj7i/OoCG7kZZKpDbdtqWbIKHdQEmX9HjfPFjqLs=; b=MNv9jLRJOutn4cobnwAZM+Ha5fwSaACPW+5rPrvMc9uFgM3G84l3TZUpO7d7bHogM0 xOqW/4jRoo9hTpVanQpoq62HnI7fgFPD5HAcwpHKkqneyeXJgFGVWzQIMB51Qq5OrgDq WxZWkzxrS5k0DLZZxf5Er7tTOSjE3Q/LGrHIrwz5iGQy2Fyyef8Xpkq6FtXitgXFGrnR cFYpYeIjXA1o+G0fMxz8vw2wSHl8DDFzU1amtuVqmN3G3HAXMn9Qd5hw9XplXll5Cltm DmWtbWX+mHux7HkCRAu16kT1/vLScHd0yisKKAxH2mMtcyPw3VupSkXp07oM3Svhb2jJ JM7g== X-Gm-Message-State: AJIora+u4E5G7HRcNKdTDfpgmoKJ4kFjpF6s/44lInZ1xbFohPwEvmvA sC9Bgu9moVvsJjSdJvujuc8OoF9Pq0QlroN3osIbnw== X-Google-Smtp-Source: AGRyM1sl5sOX7JmgxR5F/BN8PFjMkojxTysg5mhGPLjYrm6FdcTrt8XJ+wn/1bcqQfYWwWojW0IWs8oU/FKAslAidPA= X-Received: by 2002:ab0:1c56:0:b0:384:cbd7:4329 with SMTP id o22-20020ab01c56000000b00384cbd74329mr3889572uaj.9.1658975857511; Wed, 27 Jul 2022 19:37:37 -0700 (PDT) MIME-Version: 1.0 References: <20220727090700.3238-1-tujinjiang@bytedance.com> In-Reply-To: From: =?UTF-8?B?6ZSm5rGf5bGg?= Date: Thu, 28 Jul 2022 10:37:26 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] vmscan: fix potential arbitrary pointer passed to kfree in unregister_shrinker To: Yang Shi Cc: Michal Hocko , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658975859; a=rsa-sha256; cv=none; b=064SbIMpaBhaNU/Xjw2Gw6uiBl4sVFMl5iYhODZlxhEDdtv7waVIPCG0DAu1ysquPo7Byx /lA+kao+JaxcXprYvJMJVKv2sJak5ahATsVGWDs5Bd3qstTnug/g8899oxKEDjoUnwU3PV rgACb6ZSKGuFjU084jAakM76641GxpM= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=fRFnidAB; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf23.hostedemail.com: domain of tujinjiang@bytedance.com designates 209.85.222.66 as permitted sender) smtp.mailfrom=tujinjiang@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658975859; 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=MfJSj7i/OoCG7kZZKpDbdtqWbIKHdQEmX9HjfPFjqLs=; b=JBwgBeANvCH88V/U3ga9qWg9GJsr3XKbeIW059nGObd8rSYqOTyK4BXJ1s7dBEUjnmqL7P pZt5r1Zmp8e0zjLZS8llTpcQ+0OrmmjKoQ8RRpR/wyJvyL3NEOFhgu+WH8vkKxWBUeH1kk UOXa68OF3IyqkhtanzcwhVHsUjQE88U= X-Stat-Signature: 758u1awqihreu5p5g8siwi1d5qzeja1t X-Rspamd-Queue-Id: 87A2C140012 Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=fRFnidAB; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf23.hostedemail.com: domain of tujinjiang@bytedance.com designates 209.85.222.66 as permitted sender) smtp.mailfrom=tujinjiang@bytedance.com X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1658975858-656004 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 Wed, Jul 27, 2022 at 11:50 PM Yang Shi wrote: > > On Wed, Jul 27, 2022 at 7:43 AM Michal Hocko wrote: > > > > [Cc Yang Shi] > > Thanks, Michal. > > > On Wed 27-07-22 17:07:00, tujinjiang@bytedance.com wrote: > > > From: Jinjiang Tu > > > > > > when shrinker is registered with SHRINKER_MEMCG_AWARE flag, > > > register_shrinker will not initialize shrinker->nr_deferred, > > > but the pointer will be passed to kfree in unregister_shrinker > > > when the shrinker is unregistered. This leads to kernel crash > > > when the shrinker object is dynamically allocated. > > > > Is this a real life problem? I thought shrinkers were pre-zeroed > > already. Not that we should be relying on that but it would be good to > > mention whether this is a code fortification or something that we should > > be really worried about. > > Yes, all memcg aware shrinkers are actually pre-zeroed. The fs > shrinkers (embedded in super_block) are allocated by kzalloc, all > other shrinkers are static declared. So I don't think it will cause > any crash in real life. > Yes, the shrinkers in the current kernel will not cause crash, but a new memcg aware shrinker may be added in the future, and I think we should not assume the shrinker is pre-zeroed. Function free_prealloced_shrinker does not assume the shrinker is pre-zeroed, and does not call kfree(shrinker->nr_deferred) if the shrinker is memcg aware. So I think it is better for unregister_shrinker to call kfree only when the shrinker is not memcg aware. > > > > > To fix it, this patch initialize shrinker->nr_deferred at the > > > beginning of prealloc_shrinker. > > > > It would be great to add > > Fixes: 476b30a0949a ("mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers") > > > > > Signed-off-by: Jinjiang Tu > > > --- > > > mm/vmscan.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > index f7d9a683e3a7..06ab5a398971 100644 > > > --- a/mm/vmscan.c > > > +++ b/mm/vmscan.c > > > @@ -613,6 +613,7 @@ int prealloc_shrinker(struct shrinker *shrinker) > > > unsigned int size; > > > int err; > > > > > > + shrinker->nr_deferred = NULL; > > > if (shrinker->flags & SHRINKER_MEMCG_AWARE) { > > > err = prealloc_memcg_shrinker(shrinker); > > > if (err != -ENOSYS) > > > > You should be able to move it under SHRINKER_MEMCG_AWARE branch, no? > > > > -- > > Michal Hocko > > SUSE Labs