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 A5F49D637A7 for ; Thu, 14 Nov 2024 00:28:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C2346B008A; Wed, 13 Nov 2024 19:28:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 84B216B008C; Wed, 13 Nov 2024 19:28:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6C4E86B0095; Wed, 13 Nov 2024 19:28:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4B5696B008A for ; Wed, 13 Nov 2024 19:28:54 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id F2EA3A0CAB for ; Thu, 14 Nov 2024 00:28:53 +0000 (UTC) X-FDA: 82782814548.04.68BEDD1 Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com [209.85.219.45]) by imf19.hostedemail.com (Postfix) with ESMTP id 7FD991A0012 for ; Thu, 14 Nov 2024 00:27:57 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WSRFHK+a; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.45 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731543896; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Hr7YIzXk3s2XvS5AA4igZXJDd9w/u82R8OHVyJVy8Eg=; b=40Pr/aseYQi5oan9ifvIVrosVhcWhg8I/rSLQPK5ITROjhYS6lUj8I4/b1RZj6WLv0NR/H zLmB7hNRhqpl4FRUNWkoL4EDU7TGWU0NJ/cYHBNf6xM6TX7IrblXXNBOluwv8YFsLmutJ8 o65FU+NMWEQdigpxOeg3FlraEDr2qxI= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=WSRFHK+a; spf=pass (imf19.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.219.45 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731543896; a=rsa-sha256; cv=none; b=e7c/0C8jzKTteOgnzUApHYvk98gUaB0CY8KKYN75ODpTjg+Czg6sowiatLUw1v/UQicySv 4quIm8uMwzY6cnGg/ZYHgL62xQXgyhJ8oU63sp9XN5r+DV/zNqKkihUGdbrDLCuWX952ML l5Y3feL2RqLArfT7eDQqK7Dmwhex5EQ= Received: by mail-qv1-f45.google.com with SMTP id 6a1803df08f44-6cbce8d830dso71296d6.1 for ; Wed, 13 Nov 2024 16:28:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731544131; x=1732148931; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Hr7YIzXk3s2XvS5AA4igZXJDd9w/u82R8OHVyJVy8Eg=; b=WSRFHK+au+OjYL/wtDW9pq6ZUA4r9dnS3Yy0lrvEGTp9+FQ6YyvBNJHLa6UcY6MuPy jvGrKV5Kb/0nBWnyPayAhSAHN4ceXgSYxIq1GyCtU6BO/SY4kvB3NZwNVGamDPNS65cV IUMo5jBTeDQHCALepI8bZLzKMuq05lZa+m9S536WpGFMJbK8xFb/Tzs/dq5f+cTnpW2Y 3qksaFcXYWiUoZP1AEo1jXNqGW2ybwMjo/DydN1xNVdlLPD5oJf2GGy1CTQZuGEwgHwE /3izRjFVEnXLRGvXVUN38QZj0Jh8yt8SjyR8xy7kHZYQ53C7YP9G4WEZZiht6r5ADiJC 0K0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731544131; x=1732148931; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Hr7YIzXk3s2XvS5AA4igZXJDd9w/u82R8OHVyJVy8Eg=; b=FzhDIvcTlxN9OTt4CBRRejcDQMfuVUrzvFs3hJls94bxOW85SqcLNTz9k1+r0knVjQ CsMFRlmol9WNqfDGhI0fTE73IlhjrRrZ9UK0X6jvqkoszpTZ0z6s89o+k2azHFZIdXLT pmApv5lVbUR/9KcJDSKXZxzqIGiAFebonKAYmnuxU6IWj7kF/AzREJHI0vUMzJi2Iqxz Oe/R/eG1Pfum4YpX3vwAY3pkqmvzsnppLLCp+LETZ6KOBLyLuY2Xz8jo0h1v+ZoGjVyH xbh1auJOw7G30PJIx0+BxqfiePi1/mi/N8uXDZk/CtgM8KtCdite7Lu/ESkGSWm7xRuE 8p3Q== X-Forwarded-Encrypted: i=1; AJvYcCUAzMypaCXECAdqZetJxnXcNdBMKjhsdgks3dsW2ibkhZxt+6M7bAIPylHbrZr+NDxUWvaTm1eLug==@kvack.org X-Gm-Message-State: AOJu0YwZIFdylJjkboVSIEREy3w8jBSFrYzXzloACIc78Hgn16RKpBbE kTEUbAiA6NVx4LBfjempJqYpWBrWgq0em3uY0/JmUM3uRaHAKHXkbEjbb1oJgHTO8UUlkkijwbY ZD/Ehqn5QIz/kS3bmWQ2MoDqQA3I= X-Google-Smtp-Source: AGHT+IH37AfNakmSO904tMCOtS7/4FIplAgusUqyqlD0sbo5ehHehnPBigMlVtnNJ7SbzKJhydXCtoIhLEUsKY97LiI= X-Received: by 2002:a05:6214:5c09:b0:6d3:a772:fe64 with SMTP id 6a1803df08f44-6d3edb3bbf8mr1265396d6.0.1731544131174; Wed, 13 Nov 2024 16:28:51 -0800 (PST) MIME-Version: 1.0 References: <20241113052413.157039-1-kanchana.p.sridhar@intel.com> <20241113213007.GB1564047@cmpxchg.org> In-Reply-To: From: Nhat Pham Date: Wed, 13 Nov 2024 16:28:40 -0800 Message-ID: Subject: Re: [PATCH v1] mm: zswap: Fix a potential memory leak in zswap_decompress(). To: "Sridhar, Kanchana P" Cc: Johannes Weiner , Yosry Ahmed , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "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" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 7FD991A0012 X-Stat-Signature: 1pnhe6ybtcd4bqq4nzbjxyumqht543mn X-Rspam-User: X-HE-Tag: 1731544077-655162 X-HE-Meta: U2FsdGVkX19SW5+kyFROIrCIk8UtZEnP7R1j5C5CvDsigvOV98w8bq/3Oft34Gp95Ohy0EZbsuo+/zDYqVzD08X9aaJ44wWEfLupyKKjVwSzzemgjaMA76FyqsIjJc2Vl5vXCNi5nt7ZskNBJG6x3oWnBopRiRzdGktAICSlEbs2oUuaHFRtsLVQh6pLhnDuglztUM+XU/qKpyJNCulFMonnLwk16bBBMi3bnrVbFXkMF8lFy/W+gTfADcQX2OCJCZ+V0/yvi7dz5F8bs4FRp3N1GxHrqwxknc6bxxwsePBD74Vfc2ZTc8BuptnGhjg6yrX8rf2AWlCTVWPAao9ZnPeilhZxH9kMKCRe0Em8j6Um16AQcmv93DKdoefw1sRaqq1SJqbFPKiWJdL9GNjdXAFUlBK6m0Th8KdRU3KffdpMr8in/t4eRUH4RLjh1b6A1wLdNOo+uAGcFosVrOXrc5lU5JBmF4WZDT4zCcrAuSfp3G5z0i+9Rt2H/A1+a3ckPM200IU/G5xh6klUMFt68nYHLyAfruUoOZXVN4ZEsDURhtdrYr2loAkLLbm/5eKlIC7BYHneW+Jw7/yWfUXLmy6W9URFTHZp4+r3xTknzG04blOa4Jp3SVgrkj/GJkgi0eCCqbeDZv4r8qn+K2QlMpXwG17SOl6ysEx7NkcBMahIQLEL71uLeeIRukUdVW5tf8a5/a0oWhdruBIyslR/vvxsI0XYqK3QzYgG82r9mXBEHF/dqZ59b4RrZkstwHrdMxlKHJBWZSn2AA7AhDX2jcxv9odUy5+h6fofwpZC5ktYojDBguVBVscDWPsCPnKk9iSzJwynaTTAmdlwZ1hB6tcB67hJdDp8DsOOFQP594G0ML2gQ7yzbs4i3u5qXJJi4l2WNc4QztMx1wEhT+1qCP5z9VXyB39odFub9aw2M5RRiW5n4k/A4tTuauvnyyc2+itYTp1wADTFrCS+4+P JFeB3pyI yHkS3Yq2rkNXEnb9jE6oTv/EMNv64EUkazGd6M3qWBzPpvmERTEqV5YQEnrWdsc3OohTGwL42hw2vbiRAe7C+YqAtw771MTC51ghvV7zRzmnVLQIRgqVZLJper+6oFxECe8qcJIxeHAJZ+tQkz+WY3EsKGxzec5hhC4u4peVoJM3koYK7upkWw3XQ7Pcd4cT+nZWMiJS+Ye+JtnCVO3RoHURwZYld4ep0IQAlXLh5fa4Q0Ua3sz5tg6S6UMwQLtmQnkxKhNw7s1aDmA0wn/4rFPDpWvm/ozR37jxbRmNotRmqtuQ+we3FL3uPqKJhzuuG/Hti9v8Da0dwqZgxKw2bxF+GocXyxy/UCeMVJI4TJMfrlryBrYd7eSBO5qOvms+y69jcyJW7HFtsgXTRxkh81llP6CcviRIMUXOXlX5AnhVhvhr0hWVWsIJm/w== X-Bogosity: Ham, tests=bogofilter, spamicity=0.003139, 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 2:13=E2=80=AFPM Sridhar, Kanchana P wrote: > > > > Thanks Johannes, for these insights. I was thinking of the following > in zswap_decompress() as creating a non-preemptible context because > of the call to raw_cpu_ptr() at the start; with this context extending > until the mutex_unlock(): > > acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > mutex_lock(&acomp_ctx->mutex); > > [...] > > mutex_unlock(&acomp_ctx->mutex); > > if (src !=3D acomp_ctx->buffer) > zpool_unmap_handle(zpool, entry->handle); > > Based on this understanding, I was a bit worried about the > "acomp_ctx->buffer" in the conditional that gates the > zpool_unmap_handle() not being the same acomp_ctx as the one > at the beginning. I may have been confusing myself, since the acomp_ctx > is not re-evaluated before the conditional, just reused from the > start. My apologies to you and Yosry! > > > > > 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. > > I see. Wouldn't it then seem to make the code more fail-safe to not allow > the migration to happen until after the check for (src !=3D acomp_ctx->bu= ffer), by > moving the mutex_unlock() after this check? Or, use a boolean to determin= e > if the unmap_handle needs to be done as Yosry suggested? Hmm does it make it safe? It is mutex_lock() that puts the task to sleep, after which it can get migrated to a different CPU. Moving mutex_unlock() to below or not doesn't really matter, no? Or am I missing something here... I think Johannes' proposal is the safest :)