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 10961C4828D for ; Wed, 7 Feb 2024 03:28:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 808FD6B007E; Tue, 6 Feb 2024 22:28:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7B7A96B0081; Tue, 6 Feb 2024 22:28:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6A72D6B0083; Tue, 6 Feb 2024 22:28:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5CF856B007E for ; Tue, 6 Feb 2024 22:28:57 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id F2A58A0142 for ; Wed, 7 Feb 2024 03:28:56 +0000 (UTC) X-FDA: 81763576272.14.55FAB3B Received: from mail-yw1-f202.google.com (mail-yw1-f202.google.com [209.85.128.202]) by imf19.hostedemail.com (Postfix) with ESMTP id 4CEC51A0003 for ; Wed, 7 Feb 2024 03:28:55 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=RduKpk0l; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of 39vjCZQoKCCkdTXWdFMRJILTTLQJ.HTRQNSZc-RRPaFHP.TWL@flex--yosryahmed.bounces.google.com designates 209.85.128.202 as permitted sender) smtp.mailfrom=39vjCZQoKCCkdTXWdFMRJILTTLQJ.HTRQNSZc-RRPaFHP.TWL@flex--yosryahmed.bounces.google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707276535; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Cyw1f/YCIrgyWV8QmoyDjzgKLS5MFEQ8Je3Xn9KP80Q=; b=8fkV98hBCW/q0SjT3fjU6Id9jV8Fe3Ye1/22NIKnwTzw+N2s16WDhwFfVihMBUnDUbkKxb mYbEsqfqDza8qUnpJo9f2fb2kaYuDZlp4BLO9RQGiP+WupxS+2ghgf3VXihwl+AJCH8vTK b6i25ns+ZUp8yzWgLhJsxL6xQAKW7KQ= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=RduKpk0l; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of 39vjCZQoKCCkdTXWdFMRJILTTLQJ.HTRQNSZc-RRPaFHP.TWL@flex--yosryahmed.bounces.google.com designates 209.85.128.202 as permitted sender) smtp.mailfrom=39vjCZQoKCCkdTXWdFMRJILTTLQJ.HTRQNSZc-RRPaFHP.TWL@flex--yosryahmed.bounces.google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707276535; a=rsa-sha256; cv=none; b=8Jf+hRT1qs0Q1NmKrk3dnM29KuZkHmKTqy/mqrG09k9VUk0prZ/zhAEZ27qqPoPgSixjKY Vy1NTpxFe8IjnP1xzI/mXX09YbyyYN6IiatmErFtIkBKkDBhOdTLCz6Y1GMNfCAeytJJAO qHH9mkprJI0C/rMajWI2nHVUviEYK0M= Received: by mail-yw1-f202.google.com with SMTP id 00721157ae682-60484dba283so3685527b3.3 for ; Tue, 06 Feb 2024 19:28:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1707276534; x=1707881334; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Cyw1f/YCIrgyWV8QmoyDjzgKLS5MFEQ8Je3Xn9KP80Q=; b=RduKpk0lRICWg0t2T/klGIbwyp04F4+lmGuaLq6xhkNFb51mHJlXDa6tVH32jx1Z7A rvBfzdw83vJCdPq1LlWtc1c7xObIwPzBY4dRnyUlaRlSAwqjPrua2lLNwUoFRJX0379W JoAfBPof17Mt7utECekwUI7+KkKIFNNHMNLCJPt/K4dR/4LFzqmV3lrIHnNqoktcIviX 7YxdGuqQtQqQH+wtP5M62KIzTHvw0ZSTYN593najUCBgp7SWCMqXTTptf1WnIM+33JkC 9KKypu2PNiV97OXkArkIdqP6HiBfp2Qo2dDbxEykhMJpUQGf+gyG3yDSCeJPdm9t6W4+ QZNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707276534; x=1707881334; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Cyw1f/YCIrgyWV8QmoyDjzgKLS5MFEQ8Je3Xn9KP80Q=; b=c6A3RcV4KWEY3wjN2oZ0/lXxqvsLzrlvp4QyegxZvFvFVrI1UxyMtwrSUHhSOB3lc8 vEA1cqf1WANM8Y+8IozRfOLhxlC5lEJuRPuYqOXPppQli4RVIEl4ELGo06jtvanfGnBy tQAguq1X3XSIeWangdsvViJHm9Agz0o+V4WS2I/rqoTx6A+korHgnzGeg+fTgDXVIg9t qaaN9ugbeKw/ulhDyz3HP9zlTOOPMy+FEcIPIq3A9G1Qdzy67T4BZfajWYJx4qRHzGvk jb/RcDsht0239NHdVMVMlYgeEg8Yf/aIPH94kEEavae0u06+VWuYM+0VdNAxt4OQ1zL4 ZMdQ== X-Gm-Message-State: AOJu0YzEwmGj17WL7mydG+PZfPZNNjPr3HU5u2jSDb5EmgbekuC4pKuv RiInLk8t1zPantsaPp+S/5hbrKTOQGZ3wU77tX0wQ1ZMS64zj8DIwqBkJK0mAWh2EXcJP7sbwI1 zmW9L1uCwxlpTsS+eXw== X-Google-Smtp-Source: AGHT+IEHkYD1LR+yO6r3NkqTM3Vgwkf3EQ+SVotOa1AJY/hml3kRuyTUAr+YK/qyoFNh3tQJeMV2CFRbosFErOkA X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a05:6902:1b84:b0:dc2:3268:e9e7 with SMTP id ei4-20020a0569021b8400b00dc23268e9e7mr148078ybb.10.1707276534388; Tue, 06 Feb 2024 19:28:54 -0800 (PST) Date: Wed, 7 Feb 2024 03:28:52 +0000 In-Reply-To: <20240207025827.3819141-1-chengming.zhou@linux.dev> Mime-Version: 1.0 References: <20240207025827.3819141-1-chengming.zhou@linux.dev> Message-ID: Subject: Re: [PATCH v2] mm/zswap: invalidate old entry when store fail or !zswap_enabled From: Yosry Ahmed To: chengming.zhou@linux.dev Cc: hannes@cmpxchg.org, nphamcs@gmail.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Chengming Zhou , stable@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-Rspam-User: X-Stat-Signature: e88f1kwqufeqozgxyfqocwegnamrfe8h X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4CEC51A0003 X-HE-Tag: 1707276535-426873 X-HE-Meta: U2FsdGVkX19+ZLpELXPp90YTX2IM0J8ILsqcAOZecEE/rRvwgbE3Dvv0FULYV5PgItSdpWsqoPNTOZS/BW050ktnCK5KK7GIdmbJBJD/96t9oYJwGSm7ndg4MSQ5AR7FOD+tCQGNTv9VT0ansGxJOyrpdVZle2RnrQsk53TQUAFpCVw6D7GLhch7Kb+lrZTbqIDXY21zYOgFz6R4LNgYJIsvxM7osjPVh+RVzaUebKXK1QeW76OFnzTGC13y7v27zNM53OJ3I6vo+vnH+7gfNQLkELDIvAptYdXJSPCueDUlOGjWIaN9UFr6joE7FjZxzPKPOhH1/xymxrsR5SF+9j1q7HbrUETjlYQbnblluD1M7MR351Y1YUsMMeMRYGjrZ1Oz4QIIEzA9VW0J2fsMayhN2UFfN99SpzyZG3dyAHNJ3fKeUnZuxubpkHYhvbIBpu0rCkRLZclRHlrPz4zzHcehGD7MzFS0LywCL8oXf9YNnrVAFpMr+oq1OYp3C98ocNTZU2JdXZdXIZQCuth21wqDm/EPymLfRdcNHAMwkjMLYJAMTSejb7rjreKmdRnuJP6Mo69rJwIAlFcKkBpK9YzJqYrDYTDq3DcUpBpDx8E8n3EP64LCb9vpeqfyT7iATXqwAir2f1CDYCEg2og5jwCI0PkUEzsPg8770T/MDfswfqros1UAVGYIFWVbt4KB/iSTO5mUOtsGetYxGE4HiJN9WG2cIZAJkijNYYr6kaxerfzWmdwB2ol2jSGwoyVcffbHWZqY55AienpTdIUwxPbW2FtRK/+fd1ei5nXIhIG46rU6UUgqtn5sTBTrkd/neSw5Xkmx60vPWDgZHjzVVLVaNOCkmi/B3Y4T5eXuBtBISdfF7Z95SJwt6fdfWM+lXC2wW9IUn4iCSLjTHJ327OQhs8YxEZ16YrlRCmnFcE4bJkpVFuGXdkeBmwI5GYBfU5s81Bvh4xhdNx1dAVm J3Xn1snu N9CfDm8DdRWzU69UJ5Cd5s49O1wt/6740VnVPOF4voZDlR/s3lfEYlsfYwSLAqh5Yn3CassTeW1i4sHc3qwtmisioczBlfi0SqUO/1xqn7no/FkxbS+Xty8mnt6Gi+haGzbb5WhLkfZM2du7ADbMA2E9rAfjkLDNMZZi7zbaKJZ3liJ6o9GLEr4vr3Efyu0/MKm8mfgPtlF9gnEPTtvqTSY8fTUjhgPmzGqQUC7ZlRd5QtiM97LQUa+zZZs+u3kKfi1TL3GFe1Zc/nH4Our41hJlc8ZqPkSfSO/cmEpurFxvm8rcNfEGv3RaR7CcYZJ8T3X/nNfgw/63Agh+ZRWPykLZqj/yYFJQ34QPg+kV14rpqvh4IIS+bIDyF2NsSHTCfVOO3oAuZnxR0nQLB/ujl8sWr88TKcDihsmXCz/8m1Cb5PbJr7Rm84DzLwkExAmAEd6vOBpFb4QXY+Wu5fthOG020KLEG/xEu4N4xP6tfCqkWnZmIq9pfwLZceLyUPydqIvHkiAkcVTLJ+C71vzrFq9eLVkFYd0ILNF0KdRYSXmuyrHIWvAcPPTllZQozGXL2OpA82gZuGECANgiZXy8rEw3ZUR5YVKzaNOb0DLiOsyMIY7BBjDiKuiIudxTOW68nGTehyzkmV0nMw5wrO4af2P7t2W4AVqYGATHRC50K7LjyWamhYIPYG0RHBvj5Y0deBGwwM0hTpNDpp4eMDMGj4LeC4JWGREuLD/AhnqjiDyCimUwRjJ4jyY5SVsCMqj9IdcKH1N2wwVtsuLbdr+MqQLKgReS/cP5Rah0qpCtiZoVL/ckubhAjowZUp/0jVWU965/RGjiXcN6w0S0lmCiP12lBJM39/bg+26FRS5x4eNzTgncYKFl/cGaNQ6iTpwfxpgPAeJt08FpuCE4Ga76mOm0BCg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000031, 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, Feb 07, 2024 at 02:58:27AM +0000, chengming.zhou@linux.dev 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 > Signed-off-by: Chengming Zhou LGTM with a few grammatical fixes below: Acked-by: Yosry Ahmed > @@ -1608,14 +1598,12 @@ bool zswap_store(struct folio *folio) > /* map */ > spin_lock(&tree->lock); > /* > - * A duplicate entry should have been removed at the beginning of this > - * function. Since the swap entry should be pinned, if a duplicate is > - * found again here it means that something went wrong in the swap > - * cache. > + * The folio could be dirtied again, invalidate the possible old entry > + * before insert this new entry. /* * The folio may have been dirtied again, invalidate the * possibly stale entry before inserting the new entry. */ > */ > - while (zswap_rb_insert(&tree->rbroot, entry, &dupentry) == -EEXIST) { > - WARN_ON(1); > + if (zswap_rb_insert(&tree->rbroot, entry, &dupentry) == -EEXIST) { > zswap_invalidate_entry(tree, dupentry); > + VM_WARN_ON(zswap_rb_insert(&tree->rbroot, entry, &dupentry)); > } > if (entry->length) { > INIT_LIST_HEAD(&entry->lru); > @@ -1638,6 +1626,17 @@ bool zswap_store(struct folio *folio) > reject: > if (objcg) > obj_cgroup_put(objcg); > +check_old: > + /* > + * If zswap store fail or zswap disabled, we must invalidate possible > + * old entry which previously stored by this folio. Otherwise, later > + * writeback could overwrite the new data in swapfile. > + */ /* * If the zswap store fails or zswap is disabled, we must invalidate the * possibly stale entry which was previously stored at this offset. * Otherwise, writeback could overwrite the new data in the swapfile. */ > + spin_lock(&tree->lock); > + entry = zswap_rb_search(&tree->rbroot, offset); > + if (entry) > + zswap_invalidate_entry(tree, entry); > + spin_unlock(&tree->lock); > return false; > > shrink: > -- > 2.40.1 >