From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f44.google.com (mail-oa1-f44.google.com [209.85.160.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 97FC630E828 for ; Mon, 9 Feb 2026 22:50:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770677422; cv=none; b=jyDzuB0y2OLses2tgRIGmsTBUCHeoVdILOILXBEsBEYbJ4dZqN0aUiHoJOVJxZYjhEn9zFG2GOKAIhCBIAui40huvIr9GOtQdLqZeuE2MsS2PYGdB2UZlc6YLcXBZRUwVs8Z3hvYzuQg9ocklOf2R4M7p5I6TpEQGox0nFRNyKo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770677422; c=relaxed/simple; bh=dgV6NS7eIif6mbkRGeNf3teVKGdC6ZmcoQUNL6KdIpE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=c83vQiioH6WgAjPnRdTsAEMtv3PwpzPBIiz80C66H52LXMI3EDzDOvaPy1gbhxeks+kIS7ROWqQuIrGZki6xDbAiyGILADapDDRPM1nl2aZsvW0yl+/FZQKZ+02MTXzxYX5G2TfL0Afb8UUGPGS5G6q1FZCYfWv5SRDg1hIf4CQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=c+dQAJaO; arc=none smtp.client-ip=209.85.160.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="c+dQAJaO" Received: by mail-oa1-f44.google.com with SMTP id 586e51a60fabf-409470ad5bbso1312620fac.0 for ; Mon, 09 Feb 2026 14:50:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1770677420; x=1771282220; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=xA4dh7jgOju15KIFAKWH2cMywlL7aEgIic5gcSPDokc=; b=c+dQAJaOVhT5AuVdiJb9G8wxGB/NCygxz8Ryk1Nv6ipYw42IzdcPuz11WB3dwMLAbR bwrQI0H0nqdZi30rIgXw2RhnfFlMH6n6jVqHt/LCd/WX1/NCiKZi76Cp2sC2+IPgfF7d lO66Sa/+62AMSooirbLhsFIDudZn+OBAum1xJ3hlqQL1+IAknRNRuHtenjvcZmIwo9Vv o2IE6sBaeFhjAvreRGVk0fMaRvMaJidMyoZLOqneJeacKFrYeCkwmM1kQG55rhK1u5Tr yaL0uEOBPfI32IiDJ0ZPqHnfiFuCHqb8TA+zc8C5PF409w7JH85cpKubKgX5L0veUnpz k0AQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770677420; x=1771282220; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=xA4dh7jgOju15KIFAKWH2cMywlL7aEgIic5gcSPDokc=; b=fdUf6XUkqI8WX9h1Nv3AvxNF/mCSPX1cyPLDn4ngrDqYSIdTHkaC3h5aNQZyvhiZVm EoVhDzt6AsfnaghtatDO9LOnv6YEZfnuRzd8oqv74/5B6SsIdjb4PZ0geWg7lcSiUych a+BtQXVm6z6mOH+oC9mXNiSHkImHJ8imGLGz/uz9QAdRMCQlWrmyLJ0+PXHPFKV1CVkH KtsPRi4f6ZM2koSr6ymZ+L4q5MWSyazqktHqj3k+c+AB6R2UIrSW5MMdyNcvepiOJyrr 8xFCpKq/z3XcNG/cxuQn1RypJuZt6oOUGNJBoI/yu9vnipsZ7GFCkGYtUNp94g97xJP+ 9v4A== X-Forwarded-Encrypted: i=1; AJvYcCXEyZGc7zWgtL78TvTfm2jgxzFRS0vE6WEF6zydyRUkd99EmgyI5xIkWe3SmgqGfIJVxwRnhuMo4w28esw=@vger.kernel.org X-Gm-Message-State: AOJu0YyTLmyNpjRmfv6jQQdMaKf6iv9Zq2ne06mhGOQ/RZNutcqFN2xz VyJMxlhX212fxp4k37RygRHe3i403eRucUPN1jYjIVKs4qoQE8En09ENJ0qCRw== X-Gm-Gg: AZuq6aIPbKQZzMe7W40Apctrqe7V+3QiRdrb8RZbyINbrLdoqVv19WDZ2tx30kkXTdr qNrgVnjcku+peR7LoRx16VwNfQ7ByNhU8vTL8Xwr5DuY/yr1h9T8wSpHbUw/4pR+1P5jwX9qKcf dM8NDiAz09lB3hFHIlsOSDAnZ9HYGPxJDznNU7h2HA6TOQewhqjU6aqKR0DlkcyZeHFPsBB6jx7 +gHjWKODNSdvTIR3xdAULdQRQJsTAUCLZMIxvh2DEWe2f19jGxi/+0L6FRUR9rXRm2CWrRiuXi8 57nfHJ1XsBshxCUyTTWLuX9SPxHdknJj15qYgrSZ+Psep4KKzkFsmVxMDac1fjxdu4m8k4MMO83 nMybXtVRvzfshBQ0ab4KPt+5HjOF84H4d80DGQ7YWrPPAZ959sQzNOEzfmLLNR0xLp7RDvhSOre wsfz2EUHd1bA== X-Received: by 2002:a05:6870:91d3:b0:404:3ecf:9ae5 with SMTP id 586e51a60fabf-40a96efb5f7mr6611040fac.40.1770677420355; Mon, 09 Feb 2026 14:50:20 -0800 (PST) Received: from localhost ([2a03:2880:10ff:70::]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-40a99644531sm8546489fac.10.2026.02.09.14.50.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Feb 2026 14:50:19 -0800 (PST) From: Joshua Hahn To: Michael Fara Cc: senozhatsky@chromium.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org Subject: Re: [PATCH] mm/zsmalloc: fix NULL pointer dereference in get_next_zpdesc Date: Mon, 9 Feb 2026 14:50:16 -0800 Message-ID: <20260209225018.1541260-1-joshua.hahnjy@gmail.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260209193257.60393-1-mjfara@gmail.com> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Mon, 9 Feb 2026 19:32:57 +0000 Michael Fara wrote: Hello Michael, I hope you are doing well! Thank you for the patch. I'm not entirely sure if the race condition that you note here is correct, and also if this is the right fix. > get_next_zpdesc() calls get_zspage() which unconditionally dereferences > zpdesc->zspage without a NULL check. This causes a kernel oops when > zpdesc->zspage has been set to NULL by reset_zpdesc() during a race > between zspage destruction and page compaction/migration. > > The race window is documented in a TODO comment in zs_page_migrate(): > > "nothing prevents a zspage from getting destroyed while it is > isolated for migration, as the page lock is temporarily dropped > after zs_page_isolate() succeeded" I'm taking a look at zsmalloc these days, and I was confused by this comment and remember looking into what was going on here as well : -) My thoughts were that it should be safe in every other case, though. > The sequence is: > 1. Compaction calls zs_page_isolate() on a zpdesc, then drops its > page lock. > 2. Concurrently, async_free_zspage() or free_zspage() destroys the > zspage, calling reset_zpdesc() which sets zpdesc->zspage = NULL. async_free_zspage isolates the zspage first by taking the class lock and then splicing the zspage out. free_zspage is always called with the class lock held, and it likewise removes itself from the fullness list. zs_free -> free_zspage -> trylock_zspage holds the class->lock In zs_page_migrate, we call replace_sub_page to remove the zpdesc from the chain before calling reset_zpdesc, so I don't think there would be a race there either. > 3. A subsequent zs_free() path calls trylock_zspage(), which iterates > zpdescs via get_next_zpdesc(). get_zspage() dereferences the now- > NULL backpointer, causing: So I don't see how a subsequent zs_free could race with reset_zpdesc on the same zpdesc, maybe I'm missing something? : -) > BUG: kernel NULL pointer dereference, address: 0000000000000000 > RIP: 0010:free_zspage+0x26/0x100 > Call Trace: > zs_free+0xf4/0x110 > zswap_entry_free+0x7e/0x160 Is this from a real crash? A more detailed crash dump would be helpful to see what else was happening on the other threads! > The migration side already has a NULL guard (zs_page_migrate line 1675: > "if (!zpdesc->zspage) return 0;"), but get_next_zpdesc() lacks the same > protection. > > Fix this by reading zpdesc->zspage directly in get_next_zpdesc() > instead of going through get_zspage(), and returning NULL when the > backpointer is NULL. This stops iteration safely — the caller treats > it as the end of the page chain. If we return NULL early, what happens to the remaining zpdescs in the zspage? In trylock_zspage for instance, we might exit early and think we successfully locked all zpdescs, when that isn't the case. It would eventually lead to a VM_BUG_ON_PAGE when trying to free the zspage later anyways. If you have a more detailed crash trace, that would be really helpful. Thank you again for this patch, I hope you have a great day! Joshua > Signed-off-by: Michael Fara > --- > mm/zsmalloc.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -735,7 +735,19 @@ static struct zspage *get_zspage(struct zpdesc *zpdesc) > > static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc) > { > - struct zspage *zspage = get_zspage(zpdesc); > + struct zspage *zspage = zpdesc->zspage; > + > + /* > + * If the backpointer is NULL, this zpdesc was already freed via > + * reset_zpdesc() by a racing async_free_zspage() while isolated > + * for compaction. See the TODO comment in zs_page_migrate(). > + */ > + if (unlikely(!zspage)) { > + WARN_ON_ONCE(1); > + return NULL; > + } > + > + BUG_ON(zspage->magic != ZSPAGE_MAGIC); > > if (unlikely(ZsHugePage(zspage))) > return NULL; > -- > 2.39.0