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 AD44FE77197 for ; Tue, 7 Jan 2025 22:47:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 161D66B0082; Tue, 7 Jan 2025 17:47:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1121A6B0083; Tue, 7 Jan 2025 17:47:31 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF5936B0088; Tue, 7 Jan 2025 17:47:30 -0500 (EST) 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 CADB46B0082 for ; Tue, 7 Jan 2025 17:47:30 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 70809120E18 for ; Tue, 7 Jan 2025 22:47:30 +0000 (UTC) X-FDA: 82982143860.16.97CBB5C Received: from mail-vk1-f174.google.com (mail-vk1-f174.google.com [209.85.221.174]) by imf08.hostedemail.com (Postfix) with ESMTP id 7C753160005 for ; Tue, 7 Jan 2025 22:47:28 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.174 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=quarantine) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736290048; a=rsa-sha256; cv=none; b=CjvsZUrtI9V4pATFEvtvoRZ02Kd/RIz3dkunLfKJxTI539MybSAMgMZy4Z/J07sKovYcdn skqh34HJFLkNpplQlkyPTy8Yo+vm0eEzPMfvM46LG3wBPbMRPFIcOG7+eRRM4FLuFxsvSz JilB+7gedoPaObE1i+rfAhIxMM7XFds= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.174 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=quarantine) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736290048; 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; bh=3ooH0vaWtWOqcdYrhPxvWhO/7+B2PYs9eRg1ein9EOg=; b=nIMK8lTzDvWJK3s7WFiyrknvf1sVIcU1bdkYsn+fwlkyQFozCv4vr2qPvNmBIl45hDGxMk 4yEqsrxJaI5upW5YJsXj/RGgTKz4C4SZQDulE6gOCdBNv1kJ2C9kNbCctpHoIjx2uCQ03c W+4kEFwmmYAw4iYfqkqpwoRudbm/PtY= Received: by mail-vk1-f174.google.com with SMTP id 71dfb90a1353d-5187cd9b8e5so4993182e0c.1 for ; Tue, 07 Jan 2025 14:47:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736290047; x=1736894847; 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=3ooH0vaWtWOqcdYrhPxvWhO/7+B2PYs9eRg1ein9EOg=; b=piQ3nQFau0ZzmWveyayDvKrWmuAuGNy7zXSw0OuQzV09oo0tXmBMp4q2w6MCQ7dffU syu7skCNp25mtRX012ZARireM7WYcvphTl9cDBSVTii9MLEw4wVoIkcpHP3mu2GNq268 tqabZuYQ6dsMC25qR0vEyxgh34oXjxZl3o37+64m0uzbByDvA7zJWouw1VcN7Jb0rJot nF42mtg2FxJCN8Rz6eBuMJVkWginNHDFb9YAy5yTLUdVHrtOq3Ipcp2VwKtCtKVHEG/Q yxD/ADMCpXlFkTE1VxL7h4p4EzuohphiN95/rp550uRjTtggyF1H7ZwbvXWopS5Dx+A9 FhXg== X-Forwarded-Encrypted: i=1; AJvYcCWZPemp7UXt1cttuAz3QQzSsB0TU2LINz50rd1QWv05pcZ1wS9N5Oc+oGA9Dn3Qh5H5hlhd49N3Ww==@kvack.org X-Gm-Message-State: AOJu0Ywxolb9vvtsnrVjwwo5MeUL5/IdB3+NQevfOEE3AgAfWUj16iPo iRr3JjFEUZbQJaytQDTCuup0Ou3AH14fmByCtApqHuzvlB0XmgEQrQn27ZPhwfbSy/ZMcYd9QCh aOsWWAuN1eXz3WTYnQ27a+wbYylY= X-Gm-Gg: ASbGncuW/3VHDMH6MxKKSbm949i98qTcsBKDv6Y5ylwm0Zb+L/4c9S/I0ntrZIcng64 K9oqziYPXrhdhIpU0smFV/87V9B2QLRx1c5OPEIkX39d7qs4Q2mP9TWFqqzmxTfRNdWJNkBk= X-Google-Smtp-Source: AGHT+IGClFR4QNV/SEyGwTl8SkMBRo73dL+R/tN3+uEMnzHbAXKgypPI2N7hkTUZLt54KwIYmhzhTqwArZWIeu6wSVQ= X-Received: by 2002:a05:6102:2922:b0:4b2:9eeb:518f with SMTP id ada2fe7eead31-4b3d0f99b14mr750480137.10.1736290047553; Tue, 07 Jan 2025 14:47:27 -0800 (PST) MIME-Version: 1.0 References: <20250107222236.2715883-1-yosryahmed@google.com> <20250107222236.2715883-2-yosryahmed@google.com> In-Reply-To: <20250107222236.2715883-2-yosryahmed@google.com> From: Barry Song Date: Wed, 8 Jan 2025 11:47:16 +1300 X-Gm-Features: AbW1kvZP6vuj9CYsgczXGnDcHIyoKmrMcAatROq88dIGiGXVOb2cj04nfYozpzc Message-ID: Subject: Re: [PATCH v2 2/2] mm: zswap: disable migration while using per-CPU acomp_ctx To: Yosry Ahmed Cc: Andrew Morton , Johannes Weiner , Nhat Pham , Chengming Zhou , Vitaly Wool , Sam Sun , Kanchana P Sridhar , 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-Rspamd-Queue-Id: 7C753160005 X-Stat-Signature: y7uq46jbrt9dxqxgd6fwd5ey8mccabk9 X-Rspam-User: X-Rspamd-Pre-Result: action=add header; module=dmarc; Action set by DMARC X-Rspamd-Server: rspam09 X-Rspam: Yes X-HE-Tag: 1736290048-140839 X-HE-Meta: U2FsdGVkX1/d6C4VLgN+vQrakssGlOq4W23J4/DO042vwx624Z2g0Cy73ZGJORO8l178dWg251HgPrksuJLkNN4crsqos77L5FTAc8l8FGcI2u03Hn9OtQs8Jjer8jjt/8fKNfNd0s+gBrrt1K99bnBMe2O5w8SUJW766hv6erQUESTqvEHtRs9HWhyLTYkY13I/BbCQFHxIlBe7XNMcasi9yZshWECs17jvUi5vIgrvbI7wGbLGU2xzcEk+Zdh3ycms25P4K3FfJzSSGoagJbE8kNIjlLhe0QkJETvwjemhYmWsc7x1acsugar/mLeYX5z/15qrhw4gmcNFhcfJQZWNDwwrgKbYCqSN/aWnkyEQeVM3Lla49rPKC/Ki7N4FxnwLmzhCl+rV1SxTKH1iLDnFiblVKMQvefYS8tB/UNmiJGrzNZNFoRK1wq2fgMIKmnBoipVL0BTzfAOIMCnj34NZ2wDquzBflUzaaGYl999jvGRwroTZ1GR+tKhZjy5oLSf6Vp8VNJK7Gya9AXBKa5Gj07qoNtF3wg176C04d3hp5Fz/JCr6TOjT1xY4NT4EdZB2ZTJnGKWrrw36IF73UhDa3XLXC9WGxOSfF7GUOug1Tfw88GVKWvKHSCT66wBd533uWE/iy8g1kzsTxcUcLD2mlLtqImkHNQWKwQAnIK4Dgc2HNuvG2sM2Pi9fHVBmrLpDeuy5IA9sjQ5qOkg57joHEgeYv0CJrYbFsr0jZsJDV6AWZkdo9wiCldzhvJtBuINcn23mppKzqkkEUf2jplP1zjLO96FKOS8+3xOru5cn8G9cGX6Q9DGOOohIQ998etHZ3mxcRZGMVhXKaohKMyF/U9FfuhlDCTfpS+N+Rw6BGf10CsDKoVkeKFuSIcwqgR1YYv+6lJ1/rVWTzgQeR9TW24FXJqgVfAjrDiRrsxrvbq5HQ8ZONxYYaRFf1htk/tMyj39JkgM1obwYKq1 UDPQRh/k DM36jEZ451DUlNsYDXPT78CtNO+sl09O8WexVasavHOoeEFqRGadb9rF9nQLgEdUjB+f6LYl2GC5Uo2MqzBuJdxnH91aJyf1a696buMFzGqYRxTM3bhJGv8BPfZOA5muB9hhIqJRwRfL5JPbDI95i0w1H48FVvwfHjeHKsE2ufb6BxPDw7KOiNSfScpCnD9+cw5aolMH4DbbTWgBwL9g9aKOSDM/3MwQVTVsGtxbUM3hr9QOTFCohBh1QfU9O0/dt/97aAJZ1aOvMOvJPUcqjiAQHHn3S8wwZzHcQPurZSwF3B/MdOm+2py2vXYJmZxjDx91T7dnSzwMblw4qBHeF7b0z5x1zkRN/Jz0j+X8Qe/hoKoOkqAC+u34XEjZHRsMV4yYLkyv7uVm+cTBicbbTjKTBzvTNo1JcLfTXejJHCaiiekjzn7P1dEW6HnKuw/l9flOMf5NZFRHsYZXcgs1SsVB7fQ== 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 Wed, Jan 8, 2025 at 11:22=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 free= d > 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 th= e > 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. > > This cannot be fixed by holding cpus_read_lock(), as it is possible for > code already holding the lock to fall into reclaim and enter zswap > (causing a deadlock). It also cannot be fixed by wrapping the usage of > acomp_ctx in an SRCU critical section and using synchronize_srcu() in > zswap_cpu_comp_dead(), because synchronize_srcu() is not allowed in > CPU-hotplug notifiers (see > Documentation/RCU/Design/Requirements/Requirements.rst). > > This can be fixed by refcounting the acomp_ctx, but it involves > complexity in handling the race between the refcount dropping to zero in > zswap_[de]compress() and the refcount being re-initialized when the CPU > is onlined. > > Keep things simple for now and just disable migration while using the > per-CPU acomp_ctx to block CPU hotunplug until the usage is over. > > Fixes: 1ec3b5fe6eec ("mm/zswap: move to use crypto_acomp API for hardware= acceleration") > Cc: > Signed-off-by: Yosry Ahmed > 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/ > --- > mm/zswap.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index f6316b66fb236..ecd86153e8a32 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; > } > > +/* Remain on the CPU while using its acomp_ctx to stop it from going off= line */ > +static struct crypto_acomp_ctx *acomp_ctx_get_cpu(struct crypto_acomp_ct= x __percpu *acomp_ctx) > +{ > + migrate_disable(); I'm not entirely sure, but I feel it is quite unsafe. Allowing sleep during migrate_disable() and migrate_enable() would require the entire scheduler, runqueue, waitqueue, and CPU hotplug mechanisms to be aware that a task is pinned to a specific CPU. If there is no sleep during this period, it seems to be only a runqueue issue=E2=80=94CPU hotplug can wait for the task to be unpinned while it is always in runqueue. However, if sleep is involved, the situation becomes significantly more complex. If static data doesn't consume much memory, it could be the simplest soluti= on. > + return raw_cpu_ptr(acomp_ctx); > +} > + > +static void acomp_ctx_put_cpu(void) > +{ > + migrate_enable(); > +} > + > 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 > Thanks barry