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 02DBAD637C4 for ; Wed, 13 Nov 2024 21:30:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8D5E36B0085; Wed, 13 Nov 2024 16:30:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 886BD6B0089; Wed, 13 Nov 2024 16:30:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 774646B008C; Wed, 13 Nov 2024 16:30:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 589B96B0085 for ; Wed, 13 Nov 2024 16:30:16 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E0F65120E7F for ; Wed, 13 Nov 2024 21:30:15 +0000 (UTC) X-FDA: 82782364812.14.D65D3DC Received: from mail-qk1-f174.google.com (mail-qk1-f174.google.com [209.85.222.174]) by imf28.hostedemail.com (Postfix) with ESMTP id 7337CC0027 for ; Wed, 13 Nov 2024 21:29:29 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=brwu2fsP; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf28.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731533284; a=rsa-sha256; cv=none; b=Tm2JAnt5A4Jaf2Hv+EUqwW+CY5unhj/IIb5D6QUzBjGdZa5Fjk0XiBudfMimo25/+7dPMI s7qZCU/Le3nTuosAkM63Skmmj9ubEdTNlF1DaYRVUEQQJY5QApLdvdKNogzFgRK+0iMp50 zwepI9ZOOWM8W3papizPmDSemYMyddw= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=brwu2fsP; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf28.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731533284; 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=hCnYVFKewS6JOBSJeC4//MFSscRrp4qLeEy2x7eJ/Wk=; b=g6RByv+vqwixbbUTAGIuvDnH6y/16CDUx8WvFqE3jaPyXfNuwqgNxYatPULTlhs6QVKlWT hk8krPdD7Sxp5b+zvp9EKgRXGmc+bOVIRL93+JI6Wc59fCSjzzZe9CnaZn56MJ3rlkBB9I /S3gs+SYu+sEGRA3DoywEta8M6BMezA= Received: by mail-qk1-f174.google.com with SMTP id af79cd13be357-7b148919e82so489961085a.0 for ; Wed, 13 Nov 2024 13:30:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1731533412; x=1732138212; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=hCnYVFKewS6JOBSJeC4//MFSscRrp4qLeEy2x7eJ/Wk=; b=brwu2fsPHHQgMsZUh+gcfbEegj+OntMDhDWbyQYJaSM3iTe7TSz4jTqVHV26lo3hRy v7vD2dGYPT7gZKi5uxvL0xHZhVXJ1HMi37yoMp6v72/UtvkKmJSXyabscRI3i6Ejf40t 0CmnlbHN71xmNNUYjs16DEk0UTWQOh1PPgMQLEPA0bT2bSnh/F4ubFDWakvYCnpfuAkN cSGtxHP3otgDAA1oPSZkzyfyC7p+b7YRjDCOmp1/+Bj/bLbmWH+jcAb4RfnTtuTbRT2p U0wRjJKdPJ1/S8PSen3crdXKEZ7+Zij5Zf5E19MIJnGxKIjVUVjmscpZ6jVSjkOe65ZY PW9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731533412; x=1732138212; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=hCnYVFKewS6JOBSJeC4//MFSscRrp4qLeEy2x7eJ/Wk=; b=JZxSO4sVFD92B2whknaRnO9rDR4TYWctIiySkS3kuxDSslERncsh3cjj3sdenUPd8m qbMjQG2Rts3XS3iy8DA0WyNQ0Ue/ikNNsPNq4X5kfaxo4Gh32q3Th2MGtVzHTN27ZJV8 N3EfOBBV+3PDD9epddDTCy1Olu/N0TacvD1K/NmJhU9h0+1EXRrSPv21Vo8ZpAU08wtP evECPGp8VGdSbKbYRCRfcpuDNK8OvwdmjhKhJpmS0FrOIjUwxdIc6vrPxrtTwIw6zw4z bPdYXNQRYtiJIB5PPKZ9c7fc3FlMHVTJriyWwGeK0RLsThSOms/A6mSAtjwleikg906A BQCg== X-Forwarded-Encrypted: i=1; AJvYcCX5MetdlZjxqQf/Rf/+pzVvPq+aMo8dJIQVKazJazOxf9Lc+4v9nsbY+TxcUiLjIYFXLI6dJV+bSg==@kvack.org X-Gm-Message-State: AOJu0YzP7kMBa7ytU2RrqDqsAwU6QRZmFm9QXATAd576onsrXVMliFGz uj9o4h0E6vRfk2h3eJP4Up26OpN3wm/c+iIx43l+7npvScI4q58jSdChtoE/it0= X-Google-Smtp-Source: AGHT+IH2O/cl1hA6SnZJKdkbu+3Gecehn6ueBK7Ird4RKYUmg+/r+YehH5EkU1eCHlz2iVJlS5risA== X-Received: by 2002:a05:620a:31a8:b0:7b1:522a:b07 with SMTP id af79cd13be357-7b352a00c20mr512252285a.61.1731533412355; Wed, 13 Nov 2024 13:30:12 -0800 (PST) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b32acebc86sm731538185a.118.2024.11.13.13.30.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Nov 2024 13:30:10 -0800 (PST) Date: Wed, 13 Nov 2024 16:30:07 -0500 From: Johannes Weiner To: "Sridhar, Kanchana P" Cc: Yosry Ahmed , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "nphamcs@gmail.com" , "chengming.zhou@linux.dev" , "usamaarif642@gmail.com" , "ryan.roberts@arm.com" , "Huang, Ying" , "21cnbao@gmail.com" <21cnbao@gmail.com>, "akpm@linux-foundation.org" , "Feghali, Wajdi K" , "Gopal, Vinodh" Subject: Re: [PATCH v1] mm: zswap: Fix a potential memory leak in zswap_decompress(). Message-ID: <20241113213007.GB1564047@cmpxchg.org> References: <20241113052413.157039-1-kanchana.p.sridhar@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 7337CC0027 X-Stat-Signature: 9k4iubshmxg6ui67d74rtwc58zbju41n X-Rspam-User: X-Rspamd-Server: rspam05 X-HE-Tag: 1731533369-84672 X-HE-Meta: U2FsdGVkX1/zYxRiLKuZreBkQ0o/wIuefIlaHhZfKjXiBQWHtt5imzks2P896j0Y/MgvasMSqB/JMlpBmp7DCyrZYty58njARoHWIcPxbVg/iWPNdmK7qVCzAv0w8TTOoFzv3CSzLc1b54sb9qxzZMT89wrVc2XsqeOfuG4L3FCF2vW9uB1lo3xFXfuDxD/gECqnIdvLwj7uidJU8QJqDbPWKljXbCB2sGSK2ACz5eT54GKM8AEmoNwb2DmaHNu8uc7ks8U6/9XVKka8mQpzMXoE81dzECFa4GBpYLMfb+IfaX1jzrhudnSP3yp0zLzaJC1U2VEG1IxgTjTTg1dnmTh6m8IqiLP5cNQkvpderb1qnjr/wz3sJL5+VJYyVAo8eSp8yeJ0fcVyBHU24o8OhzPNDqF2VMyBjf43R0pEG4wsoZpYSA8gwo7fmUvE3OrbaBdZLPklgPXpBRofmjroCqgM/jBh71YVy1qo8s3nTGz5GAeWeW7USTUGQMHP42aXGqM8rfOdOaQtAyX+GALtKpXDhD/bSabIWVHJzvKz/9wpwZKOOyLXf4tOzOLCG0qlSwp+Vh+mwsIaGsNdqx2uMUyf+oC7TYXZ6jpJongZbodaWoT4IBgY9OzKV9Be97mO4lIEE4M9sNdxMdxHhk3GA8/vBtLdSaLIAMPq6r7Orhm6VkBFKdmTZVH9x35m+MPTM4p7ALyL+BHTY9C1/OZlaEhECfr1dTXSsEvOG+Bw/24IpmcePRtDbDpc7NUz20RIUGHz9gDqOP6B7AZOso/ahI2dvFhbCZrUCX4N8Q70wDDRhHl4RdT42WU/zClrCp7yDuWP1NPOgaJgWtNsYyl3s/skmVXPk8cnH/5FKnlocqhDW5RUfGnhFMh1jENb3V9NUYjmydAZ3aS0/D3Lslw/wtLVhO23o5BZZL/xX0BZ3cjAbdKMmI7GGbcqZ9XEdL0MSpuKqIXXuYTt3ZRPdqi c9HIj9sg LuHZ2c0NLrY6kVdVSbJ1N7XT00J0WhO/gXY5MMyspCUJs8j+cxbRzD7WUX/1sRaT7BpJ+K+pkLpCaJjSYlxc0hfhxz4lm3mFjqqfDU2dN9KfkHQ+7qMLNhndW+Thuy6++M+iMbKUoMxO7598tiYF0M4/sW1IMwSkbbKbMxLN3daVzMHDyyNX55uZI1a+u/vhGy1P3ymuoxrd2V6tEsHC6trzXcYxTHzJl/ORrlKMurodiTm1Yma/4hegSk6/pInvvuxEUyAQNABaVZMTqq+/T9wmBZT6X99hOUAGz8+i17DFPc8lchFWMrl8KgZnZfVWq2e0oqMl2XuonndE4tILq5zE8S4HLUVkcMi3sMUaYih3ltzvgQSu7vbbfCEr7gUkFggR+galoOh7yCkPXhAhtIZibcDRM03k0759fZYjIBieclSqiP25QV9vM4eUHLZ48yG90t2IP6NmsJ9hbC0OUhd7cbYjj16jp/gVEJ8/I3D/lcAg9VPGy5sRYbHHflajU+0sWrb0LlrAaQm8obPyV5H6AZ4o9K/1UHoU1CtQOqLoTqxI= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000186, 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, Nov 13, 2024 at 07:12:18PM +0000, Sridhar, Kanchana P wrote: > I am still thinking moving the mutex_unlock() could help, or at least have > no downside. The acomp_ctx is per-cpu and it's mutex_lock/unlock > safeguards the interaction between the decompress operation, the > sg_*() API calls inside zswap_decompress() and the shared zpool. > > If we release the per-cpu acomp_ctx's mutex lock before the > zpool_unmap_handle(), is it possible that another cpu could acquire > it's acomp_ctx's lock and map the same zpool handle (that the earlier > cpu has yet to unmap or is concurrently unmapping) for a write? > If this could happen, would it result in undefined state for both > these zpool ops on different cpu's? The code is fine as is. Like you said, acomp_ctx->buffer (the pointer) doesn't change. It points to whatever was kmalloced in zswap_cpu_comp_prepare(). The handle points to backend memory. Neither of those addresses can change under us. There is no confusing them, and they cannot coincide. The mutex guards the *memory* behind the buffer, so that we don't have multiple (de)compressors stepping on each others' toes. But it's fine to drop the mutex once we're done working with the memory. We don't need the mutex to check whether src holds the acomp buffer address. That being said, I do think there is a UAF bug in CPU hotplugging. There is an acomp_ctx for each cpu, but note that this is best effort parallelism, not a guarantee that we always have the context of the local CPU. Look closely: we pick the "local" CPU with preemption enabled, then contend for the mutex. This may well put us to sleep and get us migrated, so we could be using the context of a CPU we are no longer running on. This is fine because we hold the mutex - if that other CPU tries to use the acomp_ctx, it'll wait for us. However, if we get migrated and vacate the CPU whose context we have locked, the CPU might get offlined and zswap_cpu_comp_dead() can free the context underneath us. I think we need to refcount the acomp_ctx.