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 6EF5AE77184 for ; Thu, 19 Dec 2024 22:44:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D5FB66B007B; Thu, 19 Dec 2024 17:43:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D10346B0082; Thu, 19 Dec 2024 17:43:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BD78E6B0083; Thu, 19 Dec 2024 17:43:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 9EA7B6B007B for ; Thu, 19 Dec 2024 17:43:59 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 243991C61E1 for ; Thu, 19 Dec 2024 22:43:59 +0000 (UTC) X-FDA: 82913186790.26.473D898 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf13.hostedemail.com (Postfix) with ESMTP id BCB742000B for ; Thu, 19 Dec 2024 22:43:24 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="ddKafQW/"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734648221; a=rsa-sha256; cv=none; b=UJy3DbHnNoYB2WwrcasEHqDWCni0w/ISyuvSuDdbJC8fXQg266yNbnd6oWo1qfGpG76xBp a9FKOpJaK8pkfK+peWNSoSNtdz5XfbxMDZo5xgfNr0l8N1lCSz8bsF5WbTQQzOd8Q/omEL Y1uJQbcDJA52KCYuBrcjrbnnFmdOO0o= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="ddKafQW/"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734648221; 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=Ca8wF7paDL7rrljInQMjAYM9bceCl6oY8A3NHbvsl2c=; b=7BKQbVQ227B85t2OOjNqHnNjcl2BM1Vvcs40J9JQaLOqN16DCHlHB5Of1bprwGJkDi64lF CrZm9Z0NmaiKJgbbRu49FhVMluudQWQBnk7QDNPLVtC2kLlWSxaBbjLsgRyia+TXo/EzPZ EO43NasJpNWRBB8+I1Ksk4XCKb3g6VM= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-5174f9c0d2aso423306e0c.1 for ; Thu, 19 Dec 2024 14:43:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734648236; x=1735253036; 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=Ca8wF7paDL7rrljInQMjAYM9bceCl6oY8A3NHbvsl2c=; b=ddKafQW/yOOxLH2hQIrnpF0nRUANLQQAWoMqW49mC9rmwaGBToGm/NCI9y+86vtE/T a7yX94PFrDDV6Nu4qda9NaSHu1gONmlGpwW014bf/pQKBsI7qY7Rrdb6XcTOG0mwiTzy ks6b0W0xvOi02mzZhKIxkMFJVtWuS+SqjfiytwSFNPD9L9VAkkokEi5nzMfegd8TCrdE FnCoRMaMMX3ojQ6KW+Gg18YCvD9N4cP2Rpg+F5jvBliphWCJyhY5TeHJzYI7RvrSI1cv eWYVyK7ZU5ogl34/tnij0ItNcT9ESXn4c3eaWBVM8GtMjlIOmiLPCek0/jPpbmW5VqIm /udQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734648236; x=1735253036; 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=Ca8wF7paDL7rrljInQMjAYM9bceCl6oY8A3NHbvsl2c=; b=WIw3XUeOLbfQ9DocuT4p9Z+UEVRukVurTKl6lBnCBp96Cgvd0I1bB0jAReOaEuYCZ7 R7XP6N2MYoUza+pS/W2AU0JeR/pQVcY72HmOGgIsMKeAfpxuiMhOmu11E0UxxOc/9z9F ESBDXuJKr30/toZxfh3DuOcRtV/jks0/iEkx1BYgyUDmB80MYdvnGbEAwC+yBh6AbaHq muXTkvdAs2jL0oCiGDQmQPaE7AKTcj/p0yWRWRg0deTmvn0rzOJVfn6/7TINUofGv2cR IoGwMfdax/PErRe92xVuHzB0XiANTm6sZhfFB0phg4VORpuSXFpQDRt327P3eb0I3APD 68kQ== X-Forwarded-Encrypted: i=1; AJvYcCUIoYpOvMeLyKquF3Kb5cOuNXF9KbxakAhyEELHXO7S20e6z856YXm6BVpfDiIAGoWZi3VD3yC2YA==@kvack.org X-Gm-Message-State: AOJu0YxlN5MCSxD6Gvo2QWlTdalPJU31QIXdVl2JrJ6dtqf9cH06WiQ7 QW2K3WTSU0ZIODas6GgaDBVdOM6tFuJIf+sKUxjw94FkAlAUeIX53a4DOLorkekwWe5FxyVQ4bo gV54zkYET9LI983EYH3SXM2KKTLs= X-Gm-Gg: ASbGncueNHwnv92R20ZTb/JVkkifkt+zUlFvK+lyksPvJ1RWgCddbzB6iFVdMhhbX0P LxWDwUjjRiZHamY0wJo01e+ovXcsiW0hio6ezKCgKgy4gjb45IaxqwNnmlOHBw4q4EWo4HvCA X-Google-Smtp-Source: AGHT+IEDvOa56CM2TW90McyoCdDltvZgqDxiiO1bmboAF2fSbyceQHyzV5MQYDywsD/xk0aeWUoCu2YWZjNlF6qUnSU= X-Received: by 2002:a05:6122:2490:b0:516:1a78:bfb with SMTP id 71dfb90a1353d-51b75d3ff6fmr875174e0c.7.1734648236234; Thu, 19 Dec 2024 14:43:56 -0800 (PST) MIME-Version: 1.0 References: <20241219212437.2714151-1-yosryahmed@google.com> In-Reply-To: <20241219212437.2714151-1-yosryahmed@google.com> From: Barry Song <21cnbao@gmail.com> Date: Fri, 20 Dec 2024 11:43:45 +1300 Message-ID: Subject: Re: [PATCH] mm: zswap: fix race between [de]compression and CPU hotunplug To: Yosry Ahmed Cc: Andrew Morton , Johannes Weiner , Nhat Pham , Chengming Zhou , Vitaly Wool , Sam Sun , linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: BCB742000B X-Stat-Signature: fjshtccw69w31j6khmw3of8t79pjjy37 X-HE-Tag: 1734648204-23352 X-HE-Meta: U2FsdGVkX19JTRZuV4J1bJh0juVeA69YybrOnWvXeCLk3/9yOcXYNI3Otnd2QK6LqGEYNL3qYRfrheVVL6a0kDfZUrRISCs2GPRcBBqBkENrAVmhY7zDOFluS8XmrLQjbvYTptIF+MWG9OCdOCVdFPflr9J0P4KiWilCEnJ6C2HJN3PwDiG2OyQua2No0cYWl26EaGTSiVITK8V5LKlJpgGvuvxMDO3k1AcUWmVMWYFWcjdlqtWhaMISGunaqF071iH5/nZK6adsHC/5HkEoSF+k+a5Ke86MWEEoGqzn4MRjaW8XvV0ZcnZ0763YbbwbnH5KqgtbvJ0yvf5XVZWLyOksIuY0nk0o9+RgDe3r+6p+C9DoIzBdc0CpCt8I3a2KyIudE8EEa4LD0nYwOl+a6oxlrGPeaoR2R8n2CVgreyRWD/9nGrSfot5XATPDbZS3lFeRmT8WCrJ4Jcv/DUD4/R1XU62iSKZyRZW15FQCaBN/07t6MKXIW/i2N2un3cF5ec6tkFVzy/EYgmJdr9CER0K8w3WNzhz0YOiUI3ocKle/ZMBScms1OFv+TA6hDy2FHktVL1b5x/dfoOoH+vPaLs6IU42VV3vzCRrQJ1CaqfMNPWSG1mbEYIusSfghOg5vRoncq7s9RuDfwzA39bWCxo21jgQqY5h34N42BBUNuBllGISQUElgg1RdlonO8S3vCB0l51d9uSUHsAfZ4zXjtWX3vM0kDOKC/JJHU5OcNZn7sgB+aHchHTKg2C/ASzt7agIh1xfB4+csPVx0fKW0bmP1v8t/YaerydLH+LKq7E+GJPqAUR6jlRJ34aVRHmnp//xbWZRoP5yU9Id7hpdMUAjeaCk5HpIkmPmnffLfch5nOwpfYgzCWFaBHs+mU/aCaXtK5xJtPYw+veNHXgznP2Sm5ShsqTW8jpaIXEp/7zCVESYq+ALFhC3Jbf6j/a7jKx7zSI2ttRDRkLJO/qR xQo1+GWD SGlCylGsp65tCE8kJ7AviPIA614MnkIT33j3L7Uxf5IuGFuJREwtAcIUyr1eRpl62vsv+Pyi2GEg+wREKoxVxS1TH8lN4bX1T1RQ0tzu3MK3D4ikiYRTg6O5bwCA5FCGUuAaMw2OAqwAXXqtS21sxm4KhbGHvujcNOkdL72J3HIwS3J9N0NZqDjs1csj9ADpH/WpIddaSZi3rkA/f3wUlTowJq7tbWypYYy+NqSECspWU6rbnSSn4lSNVu+v25004ejeArFSCaiZjds0Vnba/BpKJLG/rH7IwA1INCQgTa0e8VqwQPjCL7EhoVbSjWYYb5gtH/uRklv+085ZTfNWYlxBWQ4gRFBPygCm+vXRoixLeJqefdC4LU+PZMLr/zWYt3ur6sirPdeaDnYvKSR3iSQ79dMZbk+KAKVvVMM8YncLHH157/m2zAtJlfKLW9ds0LY18 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 Fri, Dec 20, 2024 at 10:24=E2=80=AFAM Yosry Ahmed wrote: > > In zswap_compress() and zswap_decompress(), the per-CPU acomp_ctx of the > current CPU at the beginning of the operation is retrieved and used > throughout. However, since neither preemption nor migration are > disabled, it is possible that the operation continues on a different > CPU. > > If the original CPU is hotunplugged while the acomp_ctx is still in use, > we run into a UAF bug as the resources attached to the acomp_ctx are > freed during hotunplug in zswap_cpu_comp_dead(). > > The problem was introduced in commit 1ec3b5fe6eec ("mm/zswap: move to > use crypto_acomp API for hardware acceleration") when the switch to the > crypto_acomp API was made. Prior to that, the per-CPU crypto_comp was > retrieved using get_cpu_ptr() which disables preemption and makes sure > the CPU cannot go away from under us. Preemption cannot be disabled with > the crypto_acomp API as a sleepable context is needed. > > Commit 8ba2f844f050 ("mm/zswap: change per-cpu mutex and buffer to > per-acomp_ctx") increased the UAF surface area by making the per-CPU > buffers dynamic, adding yet another resource that can be freed from > under zswap compression/decompression by CPU hotunplug. > > There are a few ways to fix this: > (a) Add a refcount for acomp_ctx. > (b) Disable migration while using the per-CPU acomp_ctx. > (c) Disable CPU hotunplug while using the per-CPU acomp_ctx by holding > the CPUs read lock. > > Implement (c) since it's simpler than (a), and (b) involves using > migrate_disable() which is apparently undesired (see huge comment in > include/linux/preempt.h). > > Fixes: 1ec3b5fe6eec ("mm/zswap: move to use crypto_acomp API for hardware= acceleration") > Reported-by: Johannes Weiner > Closes: https://lore.kernel.org/lkml/20241113213007.GB1564047@cmpxchg.org= / > Reported-by: Sam Sun > Closes: https://lore.kernel.org/lkml/CAEkJfYMtSdM5HceNsXUDf5haghD5+o2e7Qv= 4OcuruL4tPg6OaQ@mail.gmail.com/ > Cc: > Signed-off-by: Yosry Ahmed Thanks for fixing this. Acked-by: Barry Song > --- > mm/zswap.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index f6316b66fb236..5a27af8d86ea9 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -880,6 +880,18 @@ static int zswap_cpu_comp_dead(unsigned int cpu, str= uct hlist_node *node) > return 0; > } > > +/* Prevent CPU hotplug from freeing up the per-CPU acomp_ctx resources *= / > +static struct crypto_acomp_ctx *acomp_ctx_get_cpu(struct crypto_acomp_ct= x __percpu *acomp_ctx) > +{ > + cpus_read_lock(); > + return raw_cpu_ptr(acomp_ctx); > +} > + > +static void acomp_ctx_put_cpu(void) > +{ > + cpus_read_unlock(); > +} > + > static bool zswap_compress(struct page *page, struct zswap_entry *entry, > struct zswap_pool *pool) > { > @@ -893,8 +905,7 @@ static bool zswap_compress(struct page *page, struct = zswap_entry *entry, > gfp_t gfp; > u8 *dst; > > - acomp_ctx =3D raw_cpu_ptr(pool->acomp_ctx); > - > + acomp_ctx =3D acomp_ctx_get_cpu(pool->acomp_ctx); > mutex_lock(&acomp_ctx->mutex); > > dst =3D acomp_ctx->buffer; > @@ -950,6 +961,7 @@ static bool zswap_compress(struct page *page, struct = zswap_entry *entry, > zswap_reject_alloc_fail++; > > mutex_unlock(&acomp_ctx->mutex); > + acomp_ctx_put_cpu(); > return comp_ret =3D=3D 0 && alloc_ret =3D=3D 0; > } > > @@ -960,7 +972,7 @@ static void zswap_decompress(struct zswap_entry *entr= y, struct folio *folio) > struct crypto_acomp_ctx *acomp_ctx; > u8 *src; > > - acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > + acomp_ctx =3D acomp_ctx_get_cpu(entry->pool->acomp_ctx); > mutex_lock(&acomp_ctx->mutex); > > src =3D zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); > @@ -990,6 +1002,7 @@ static void zswap_decompress(struct zswap_entry *ent= ry, struct folio *folio) > > if (src !=3D acomp_ctx->buffer) > zpool_unmap_handle(zpool, entry->handle); > + acomp_ctx_put_cpu(); > } > > /********************************* > -- > 2.47.1.613.gc27f4b7a9f-goog > Best Regards Barry