linux-kernel - Re: [PATCH v2 1/2] Revert "mm: zswap: fix race between [de]compression and CPU hotunplug"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJD7tkYV-=n1Zk3tQd8Y+1y7ZQ_VT9RYYqzDMGHnzK3zRmNxUA@mail.gmail.com>
Date: Tue, 7 Jan 2025 15:39:13 -0800
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Barry Song <baohua@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Johannes Weiner <hannes@...xchg.org>, 
	Nhat Pham <nphamcs@...il.com>, Chengming Zhou <chengming.zhou@...ux.dev>, 
	Vitaly Wool <vitalywool@...il.com>, Sam Sun <samsun1006219@...il.com>, 
	Kanchana P Sridhar <kanchana.p.sridhar@...el.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH v2 1/2] Revert "mm: zswap: fix race between
 [de]compression and CPU hotunplug"

On Tue, Jan 7, 2025 at 3:01 PM Barry Song <baohua@...nel.org> wrote:
>
> On Wed, Jan 8, 2025 at 11:22 AM Yosry Ahmed <yosryahmed@...gle.com> wrote:
> >
> > This reverts commit eaebeb93922ca6ab0dd92027b73d0112701706ef.
> >
> > Commit eaebeb93922c ("mm: zswap: fix race between [de]compression and
> > CPU hotunplug") used the CPU hotplug lock in zswap compress/decompress
> > operations to protect against a race with CPU hotunplug making some
> > per-CPU resources go away.
> >
> > However, zswap compress/decompress can be reached through reclaim while
> > the lock is held, resulting in a potential deadlock as reported by
> > syzbot:
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 6.13.0-rc6-syzkaller-00006-g5428dc1906dd #0 Not tainted
> > ------------------------------------------------------
> > kswapd0/89 is trying to acquire lock:
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: acomp_ctx_get_cpu mm/zswap.c:886 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_compress mm/zswap.c:908 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_store_page mm/zswap.c:1439 [inline]
> >  ffffffff8e7d2ed0 (cpu_hotplug_lock){++++}-{0:0}, at: zswap_store+0xa74/0x1ba0 mm/zswap.c:1546
> >
> > but task is already holding lock:
> >  ffffffff8ea355a0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat mm/vmscan.c:6871 [inline]
> >  ffffffff8ea355a0 (fs_reclaim){+.+.}-{0:0}, at: kswapd+0xb58/0x2f30 mm/vmscan.c:7253
> >
> > which lock already depends on the new lock.
>
> We have functions like percpu_is_write_locked(),
> percpu_is_read_locked(), and cpus_read_trylock().
> Could they help prevent circular locking dependencies if we perform a
> check before acquiring the lock?

Yeah we can do that but it feels a bit hacky, we may have to
unnecessarily fail the operation in some cases, right? Not sure tbh.