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 3ED42C4345F for ; Thu, 18 Apr 2024 20:10:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B6E846B0088; Thu, 18 Apr 2024 16:10:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B45816B00A9; Thu, 18 Apr 2024 16:10:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A0D706B00CA; Thu, 18 Apr 2024 16:10:06 -0400 (EDT) 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 818436B0088 for ; Thu, 18 Apr 2024 16:10:06 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3F52AA239A for ; Thu, 18 Apr 2024 20:10:06 +0000 (UTC) X-FDA: 82023744012.25.7D09F4C Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by imf07.hostedemail.com (Postfix) with ESMTP id 5FA1A40012 for ; Thu, 18 Apr 2024 20:10:03 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BmwKAudK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713471003; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Mp8BfU7cj3Avv6w7b6diX/CWBRB9qpT5rd/d7P5dcFg=; b=2JxVuZK2a93lFpIRpLjP4ZDNpd3ErtvmnEZwFISxeUNNtYMTmnr0ZjxaYMec5b1WcOMUna 9695+WVJqze6+rjb6G6wapIBg4IRHT9JjnVK+n+gFOy37twstMsmSw8wWXi51z/2X2aWB2 LT3d56+wv/Idqsgh56H3QD2Kp5Cy0bg= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BmwKAudK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713471003; a=rsa-sha256; cv=none; b=BJpqJf9RotjqHmhBgwBPIjZLhypgRoJfvNBjaFGBbpz6+kxNaxfqdA5Nvkf1eBxv6BH9G+ iVg92vInUoyJz6tdGOlNZhZALzPYr0SCOCXIP1YiwvYvQl0HsUesbA33tQEIY6/TDx7G2J 1nOrh5fdu4UFUP2QVCTnkMLlcb9ZrmQ= Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-516d1c8dc79so1512530e87.1 for ; Thu, 18 Apr 2024 13:10:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713471001; x=1714075801; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Mp8BfU7cj3Avv6w7b6diX/CWBRB9qpT5rd/d7P5dcFg=; b=BmwKAudKt8gFM4Ac3YUIIG0041lYv2hMbyPwNv9w7/yzemhhzLkfOyW9n1cUG4cRj5 b72Rq0Pp0CcPUAdVbAIXOpFN+Bnu7TEVy8SyjjMeDX08FDxLtjAZP0RFogqN8cW0DoCL fe0zh6g+LGMcBU0r/XUaSnxijp+HpvpcfXz6BCqJzYQ3mqdwFxl1TW25BJsOqr/6L+pL 62EjJLWchCVSM5vgGg2XTm12xx0hJeYhGRHtkdB7MC/bmsvqFUmnJD+feeDV4XkxpTL+ k9moCM0iP0wIsB6szOGLId6Dg4K3JFDrusdh6t3ggvw55a/n/zZvPtiQK87dxPv0ghqb oyWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713471001; x=1714075801; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Mp8BfU7cj3Avv6w7b6diX/CWBRB9qpT5rd/d7P5dcFg=; b=XiY/AHm3tBR5AYXq10I1jom+u0XEFb2tco4ADQ6a265Zuj6OydN4TM9p2d0dOhuT5+ VLjSGb19zrgZkHj1LkQCL1RG4ciwD4tTxSnJiau76Ig2EgRupT6Njs/vBhX1AvwT2jyD qIFSdsdG7hlRn9q9Y7AcoLb9aynOl5pxR1IX4PNMHxcNkU74yNsCr0CChGt1UDuQ8DwI sVuJkrZJWuMXRIkDEBIad/LHi6diFDsiT5q6hHIyGAs7hTLIvin+53KUodZI3ktWVDVO FKQlDCZZNutmofAVrsvavHOzqp4HYLz31cRvxNshkecSd16tnfA0wDT67/QK0y2jFYZ1 54ig== X-Forwarded-Encrypted: i=1; AJvYcCXvZzOcGtf1Sax9p6ApZ367o6asKQv/lw9aw7uWQjmRNJcqJm7fQ0igGTSfhZnehF4nzbOX+oomz5ZvaWhV1/JdZm8= X-Gm-Message-State: AOJu0YzcE765ITNLBFVWIEA0xlDp0iS63SMI6GXHmkDPk55zX8/u14nK K6C0/cuWsSfciUxujvSv3Bojfyv2GXBCXf+BX3eVrfvzFpcswMK0FE7r2jz9KBMwGu180zZCacs 5LDvljltCXePPTDpL5oZwyjTUJPe9+fv2Te+x X-Google-Smtp-Source: AGHT+IGLDG+Dc0014dtU8+cIhp81ZKUWtv/7wQd5vaFrsMLyPtvkI6wgcg66uZWMynrXXfhRZfZa+0OQy8ad/+Cb/Og= X-Received: by 2002:ac2:5d44:0:b0:518:baa1:381b with SMTP id w4-20020ac25d44000000b00518baa1381bmr46743lfd.50.1713471001029; Thu, 18 Apr 2024 13:10:01 -0700 (PDT) MIME-Version: 1.0 References: <3iccc6vjl5gminut3lvpl4va2lbnsgku5ei2d7ylftoofy3n2v@gcfdvtsq6dx2> <246c1f4d-af13-40fa-b968-fbaf36b8f91f@linux.dev> <20240417143324.GA1055428@cmpxchg.org> <4c3ppfjxnrqx6g52qvvhqzcc4pated2q5g4mi32l22nwtrkqfq@a4lk6s5zcwvb> <20240418124043.GC1055428@cmpxchg.org> In-Reply-To: <20240418124043.GC1055428@cmpxchg.org> From: Yosry Ahmed Date: Thu, 18 Apr 2024 13:09:22 -0700 Message-ID: Subject: Re: [REGRESSION] Null pointer dereference while shrinking zswap To: Johannes Weiner Cc: Christian Heusel , Chengming Zhou , Nhat Pham , Seth Jennings , Dan Streetman , Vitaly Wool , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, David Runge , "Richard W.M. Jones" , Mark W , regressions@lists.linux.dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 5FA1A40012 X-Stat-Signature: e1mjjer5344h9h6atqq3ohmcjcdydnt4 X-HE-Tag: 1713471003-485085 X-HE-Meta: U2FsdGVkX18vQ6+QdJjyRHWZ8q3JrPqscI1UF/bMMsBBkY/Fu6XeVONm+KC67701rJ20W4WwBHo4u0EgwdQHg5zgfW2QtvwbxAlRAy8oCtnrSJpcjTEYfkcPRqziavrS6sDRnVVuRC1bTxRM8Vkxmpdstc37XFinQofMjd2fM9QU9pMzrGDxvpTszKH/zxYND0aTYA9ej0L7SANgP7KDO7TswHORaOzjIss/uj9j37cilihOjrjwzuAT58GtZRr0QjFiEv3sKKgBJEEBfmMDI09/DOXZS5j9jRpYi0hLiWV6MlCvbNZKdCmOVAUDOAyF3OylMyCf+XjrFYxbSRHlQ8J0yQyQQ3t/gTvl0tUvYTq/+GgtW+l0KyVWZYYJg/5fjrC+UiHQ+o6DcTAqvnHtydmW+CPW5RLoKAfmFAGcRXyzrrUshMxrCN0jBUifT19Q8XZeulE7Ca91BzpvFW7zCm1HWj4Vp5mhO7nRdduvA1uBQKQxamLIZ8W//A+UJX7rV5tI+R4ZpJOThE5c7B5d95KndL1l8wdz4mWKTftaKLxDYVHKWIol5EHtXPUoG52BcmhlzDso0kpqn02cysIDRLQkcDHZOkziWw4zgSBub2OrSkzvlGm5SYe62LURjShQ/+X7f0np9HHEkud0Rv9QYwD2P9033hLirWLvvd34apXNaMd/4t8lAM2sOAaLgwOYlC5ujOAcEXQW0ET/fEa0XV8CKvJ2NGWXW7dSLVI3jn/3zAgqKQksq4efJ4ykj2Ig0XZ0S61EAQyytqWML+GQXSJty9WVSepbSG9jeMtE9UwbIELnP3sF0zKzhdYxK+RRWon7dYMRK9FE5QGhhmkxqfmR/Gj1ZzFyhBFE28p+6yCHJsEg4lVoS5zztmxEHxmYSY22R/jhTyhgicGLxneZqXYo4Ih80OKAUXd+SlZ4F9slWqJEVTY9RugT4B8KrI4Ccm3ARrEXgwjSsYK5wNv M6oercxU A1Jr7yvBepMKAqrhulrP6zkF9uXTIjWzIsiDJRl6Isc5JEG/ogaynalHnOrkKAl/DNt/qLC4AhiubILfj6X7HhDUu8w7KocdE2NsjNiZ/k9EAex8YsRiYZw8Vwz/kylHe9KBC7A8pKZLpF+UTL1hwkTi70J3EGxXI69tMAMBU85Cr4gkxpENUVoPBzYwMtpUolpnp3etULxG358bod9p7rAa1JgGDCtKW89GopXcLGF0b8AWAym5XmZSCaPq94rNTUpSUKDwMwH88Y5WplHiVEaxjmzBdXo/3Ia6eiNfYJL5Run8Wl4S3KWby1TgjWogqQtZvbQMVazQrl7qZ1eeuwVfACEQaIatsd3LHbG9kJCW9DMBCmOJXQuwc1hZ8Y84oOTMF3R2wWcunHiYr2h/MAcfs/rygCyYzOJeRJE/Ir9TVLUDwLqtS+4YP0yDahnTcVHtjDl8lClsvRlStH8mS0gCmmF5b9TP/S3jknp6KRlsmAa+6jdkji6RwgQ== 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 Thu, Apr 18, 2024 at 5:40=E2=80=AFAM Johannes Weiner wrote: > > On Wed, Apr 17, 2024 at 07:18:14PM +0200, Christian Heusel wrote: > > On 24/04/17 10:33AM, Johannes Weiner wrote: > > > > > > Christian, can you please test the below patch on top of current > > > upstream? > > > > > > > Hey Johannes, > > > > I have applied your patch on top of 6.9-rc4 and it did solve the crash = for > > me, thanks for hacking together a fix so quickly! =F0=9F=A4=97 > > > > Tested-By: Christian Heusel > > Thanks for confirming it, and sorry about the breakage. > > Andrew, can you please use the updated changelog below? > > --- > > From 52f67f5fab6a743c2aedfc8e04a582a9d1025c28 Mon Sep 17 00:00:00 2001 > From: Johannes Weiner > Date: Thu, 18 Apr 2024 08:26:28 -0400 > Subject: [PATCH] mm: zswap: fix shrinker NULL crash with cgroup_disable= =3Dmemory > > Christian reports a NULL deref in zswap that he bisected down to the > zswap shrinker. The issue also cropped up in the bug trackers of > libguestfs [1] and the Red Hat bugzilla [2]. > > The problem is that when memcg is disabled with the boot time flag, > the zswap shrinker might get called with sc->memcg =3D=3D NULL. This is > okay in many places, like the lruvec operations. But it crashes in > memcg_page_state() - which is only used due to the non-node accounting > of cgroup's the zswap memory to begin with. > > Nhat spotted that the memcg can be NULL in the memcg-disabled case, > and I was then able to reproduce the crash locally as well. > > [1] https://github.com/libguestfs/libguestfs/issues/139 > [2] https://bugzilla.redhat.com/show_bug.cgi?id=3D2275252 > > Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure") > Cc: stable@vger.kernel.org [v6.8] > Link: https://lkml.kernel.org/r/20240417143324.GA1055428@cmpxchg.org > Reported-by: Christian Heusel > Debugged-by: Nhat Pham > Suggested-by: Nhat Pham > Tested-By: Christian Heusel > Signed-off-by: Johannes Weiner Thanks for fixing this. A couple of comments/questions below, but anyway LG= TM: Acked-by: Yosry Ahmed > --- > mm/zswap.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index caed028945b0..6f8850c44b61 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1331,15 +1331,22 @@ static unsigned long zswap_shrinker_count(struct = shrinker *shrinker, > if (!gfp_has_io_fs(sc->gfp_mask)) > return 0; > > -#ifdef CONFIG_MEMCG_KMEM > - mem_cgroup_flush_stats(memcg); > - nr_backing =3D memcg_page_state(memcg, MEMCG_ZSWAP_B) >> PAGE_SHI= FT; > - nr_stored =3D memcg_page_state(memcg, MEMCG_ZSWAPPED); > -#else > - /* use pool stats instead of memcg stats */ > - nr_backing =3D zswap_pool_total_size >> PAGE_SHIFT; > - nr_stored =3D atomic_read(&zswap_nr_stored); > -#endif > + /* > + * For memcg, use the cgroup-wide ZSWAP stats since we don't > + * have them per-node and thus per-lruvec. Careful if memcg is > + * runtime-disabled: we can get sc->memcg =3D=3D NULL, which is o= k > + * for the lruvec, but not for memcg_page_state(). > + * > + * Without memcg, use the zswap pool-wide metrics. > + */ > + if (!mem_cgroup_disabled()) { With the current shrinker code, it seems like we cannot get sc->memcg =3D=3D NULL unless mem_cgroup_disabled() is true indeed. However, maybe it's better to check if sc->memcg is not NULL directly instead? This would be more resilient in case the shrinker code changes and passing sc->memcg =3D=3D NULL becomes possible in other cases (e.g. during global shrinking). It seems like other shrinkers do this, for example see count_shadow_nodes() and deferred_split_count(). I am also wondering if we should also check !mem_cgroup_is_root() here? We can avoid the expensive global flush and use the global stats directly in this case. I could also send a follow up patch for this if that's preferred. > + mem_cgroup_flush_stats(memcg); > + nr_backing =3D memcg_page_state(memcg, MEMCG_ZSWAP_B) >> = PAGE_SHIFT; > + nr_stored =3D memcg_page_state(memcg, MEMCG_ZSWAPPED); > + } else { > + nr_backing =3D zswap_pool_total_size >> PAGE_SHIFT; > + nr_stored =3D atomic_read(&zswap_nr_stored); > + } > > if (!nr_stored) > return 0; > -- > 2.44.0