From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-179.mta1.migadu.com (out-179.mta1.migadu.com [95.215.58.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CA3A91D524 for ; Thu, 8 Feb 2024 02:35:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707359703; cv=none; b=azm9AsuPhJIgCXhNgSwvKkztajuZfGy9uB7HUnmcEMqFwZfgJhZjn1rLr8jJv0MHIf68XsbFsZBxJuOfPHH6F3vMTofq1JFBRUydlwmiiw3OYfkvtcNanKLwrjIElyNFSfgK6d/YSKnHg5xUmT2wUCR0vR49bBWm5LN5evN54vU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707359703; c=relaxed/simple; bh=EkOqfampjeSKQg5Cr8kvbOMjAirzAY3ivCor7kCy+9I=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=AE6nQlH4f5Y5bZEqXC8dB8mg7lexL/SPJxiI/wF09TPfAp5QFXb+WNiP12AvfEC4/M4UFbkVAFtcKyxDI78wDrtMpaJUIsptzvIFXaxokE7ViGPO4NBfVJjWvAtR8zre6rZhKPUb8RoiLP7XCbEoePtj7P+22xERMCh85oHZhpY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=cSqV5VGu; arc=none smtp.client-ip=95.215.58.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="cSqV5VGu" Message-ID: <9dec8d00-c01b-44fe-983a-ec2b83d71e2a@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1707359700; h=from:from: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=JcrtKKNrzp8Qp0fqpYyaI1RyGJgRqDnnLYhAFWHoxnw=; b=cSqV5VGuSF0ia9JzWNev68oQ36rGLC/zt3lp6piRGnu4bMViCrk5P9y+85jZ0slalDg6Mv O+vWNGeAQ9r4clnhO7Q+RkdRsUrYlwCzKtU3n+A4Qho7Gqs8hFRALgr+dPdDlEeL41xNbQ 0kWe69ej4IETPXXtQwDoSWX+n78X/C8= Date: Thu, 8 Feb 2024 10:34:54 +0800 Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v4] mm/zswap: invalidate old entry when store fail or !zswap_enabled Content-Language: en-US To: Nhat Pham Cc: hannes@cmpxchg.org, yosryahmed@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Chengming Zhou , stable@vger.kernel.org, Chris Li References: <20240207115406.3865746-1-chengming.zhou@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On 2024/2/8 07:06, Nhat Pham wrote: > On Wed, Feb 7, 2024 at 3:54 AM wrote: >> >> From: Chengming Zhou >> >> We may encounter duplicate entry in the zswap_store(): >> >> 1. swap slot that freed to per-cpu swap cache, doesn't invalidate >> the zswap entry, then got reused. This has been fixed. >> >> 2. !exclusive load mode, swapin folio will leave its zswap entry >> on the tree, then swapout again. This has been removed. >> >> 3. one folio can be dirtied again after zswap_store(), so need to >> zswap_store() again. This should be handled correctly. >> >> So we must invalidate the old duplicate entry before insert the >> new one, which actually doesn't have to be done at the beginning >> of zswap_store(). And this is a normal situation, we shouldn't >> WARN_ON(1) in this case, so delete it. (The WARN_ON(1) seems want >> to detect swap entry UAF problem? But not very necessary here.) >> >> The good point is that we don't need to lock tree twice in the >> store success path. >> >> Note we still need to invalidate the old duplicate entry in the >> store failure path, otherwise the new data in swapfile could be >> overwrite by the old data in zswap pool when lru writeback. >> >> We have to do this even when !zswap_enabled since zswap can be >> disabled anytime. If the folio store success before, then got >> dirtied again but zswap disabled, we won't invalidate the old >> duplicate entry in the zswap_store(). So later lru writeback >> may overwrite the new data in swapfile. >> >> Fixes: 42c06a0e8ebe ("mm: kill frontswap") >> Cc: >> Acked-by: Johannes Weiner >> Acked-by: Yosry Ahmed >> Acked-by: Chris Li >> Signed-off-by: Chengming Zhou > > Acked-by: Nhat Pham > > Sorry for being late to the party, and thanks for fixing this, Chengming! Thanks for your review! :)