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 277EFC3DA4A for ; Thu, 1 Aug 2024 12:54:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B87586B0093; Thu, 1 Aug 2024 08:54:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B37E86B0095; Thu, 1 Aug 2024 08:54:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9FF0D6B0096; Thu, 1 Aug 2024 08:54:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 7F2336B0093 for ; Thu, 1 Aug 2024 08:54:19 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 18A05C0628 for ; Thu, 1 Aug 2024 12:54:19 +0000 (UTC) X-FDA: 82403669838.01.964190E Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf27.hostedemail.com (Postfix) with ESMTP id 4265B40005 for ; Thu, 1 Aug 2024 12:54:17 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Ef4I1wME; spf=pass (imf27.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=andreyknvl@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=1722516781; 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=oJM7ZW7Cx4ewZwlyf9TpDzuVdkBNUpK6wYJDwO0sS4w=; b=e8Box3jdI5lclO3M1+C0yRWVtpU9vbZOknohE+SLY28TIcr706N6QwfbQGpTdvELTsIYUn dk/vk+wFWmGbUUcVYcdzhZqxFzd+UeWYwaGDPHkOR4xdCEBcUAVZNTkxEdGXT3g/TTr1Pw jnA010VG+um6aLTQ/s2uk5e1kdXNmXA= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Ef4I1wME; spf=pass (imf27.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722516781; a=rsa-sha256; cv=none; b=j/PSskG/2BJ8cJZ2sIXZIm/U3zE5wPIER8/p3powaf8XIEu/9sRnWl2v2xp/8/Vdrk+opm pKa1Og4oQC/frXhCXrw88uiu5WR8LcAUCVnAZ7Wex5ya68l2OqbztQwqyBLgfEXcvD95UX 7UfyD1hJaPz0Dw/JS+9V4d5vqvla7EM= Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-4266f3e0df8so44104985e9.2 for ; Thu, 01 Aug 2024 05:54:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722516856; x=1723121656; 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=oJM7ZW7Cx4ewZwlyf9TpDzuVdkBNUpK6wYJDwO0sS4w=; b=Ef4I1wMEWX4qsBnNMZcOT0akFA6p/8uHCb7yFJBdV8Qx6ofel+GeLW/JwAzU7xVBL0 WdT8CdY/3auHBVVJ2DJl38ln+Ky0aIM1oUprFmPq7AKhKDArP0VUzd86974UsaeKfne+ RAy9UENj0KMAErpe/z0ZNX/8u09FWzcKrw/+al3LzKw2FCGsDePclTNowdQGhFuLiXsw vwRXg8CWO5ctV/Ut0/sR/Z1SMCENzYeuE8ZWYOpnisZAuONIOcjDumuAYYMg77Op7Vmt IIFdjMpaVdmv7Tdp83JK52P+LW7kCD1jFLhvbSc1/inswAMKiDDU7MYlzD5uSNvTjXOu vpgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722516856; x=1723121656; 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=oJM7ZW7Cx4ewZwlyf9TpDzuVdkBNUpK6wYJDwO0sS4w=; b=oGpe2vtJU8vTbfELLEXTY3fjWvdBGf6QoZs7sJvKHiiyqwZoKpcK8n3HWdyATVEzPv EzHyNia5SF1TwbX8Lzl1wWRRWWuinGCzpUsK7f0GAbi5a39AC2kPr8Yh0z0RQEuWvxg1 +Iw6TYNYPLbrWMkmpNWPXm99AafTInFXbcwPg2HZwiC8/XvtovhmjxCZgwX8AwSZfcKH 6Il0yNJD2xkJRhcf4IifCWH0eW0sb9lZy3Yfuk9zYOq05NQRiri7hUEQfpxC7rLMTTVN YEj9LTIoPE5ut3JxemvXZM37Kfge+hSVDk2OiywEpDW9JCgTnsdPyxKlJ37XQcjD7Xao Othg== X-Forwarded-Encrypted: i=1; AJvYcCUSeEKBMM7nRiJbFtDA85pmAgwTVr8+GujZGXx7IAsVDBWZYCX+IDs3b1G2AXnRMwrLO4PvndwLwubs1TzxKatZ8wQ= X-Gm-Message-State: AOJu0Yysqi9tHNctSBkYf6Doi2O9GbeMWd45b5T8nrAD/gvUyqt3x9+w xSm7Q9BFlCx81uUzvu7VW+E4zMIPf/Dr13o1bTq4+ZgCez22c9/qm+y8t8q5gIhv2MdXS+9r7oW XA0FwYmY1F6Y9qIo7BTecLZvYYPc= X-Google-Smtp-Source: AGHT+IEn73WxbN6uH+EfdL648nkpBQ6mZ8M7TiPsUS0OMVpuMAMF7/HRxcKTxJ+JMMtr0tPw0IRNmeYPvvqi5MnJTs4= X-Received: by 2002:a5d:62c5:0:b0:368:3b21:6643 with SMTP id ffacd0b85a97d-36baaf237a1mr1484941f8f.48.1722516855454; Thu, 01 Aug 2024 05:54:15 -0700 (PDT) MIME-Version: 1.0 References: <20240730-kasan-tsbrcu-v5-0-48d3cbdfccc5@google.com> <20240730-kasan-tsbrcu-v5-1-48d3cbdfccc5@google.com> In-Reply-To: From: Andrey Konovalov Date: Thu, 1 Aug 2024 14:54:04 +0200 Message-ID: Subject: Re: [PATCH v5 1/2] kasan: catch invalid free before SLUB reinitializes the object To: Jann Horn Cc: Marco Elver , Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 4265B40005 X-Stat-Signature: b4c54uxiaewsb6j7uopuwdd854dtwfcf X-Rspam-User: X-HE-Tag: 1722516857-322762 X-HE-Meta: U2FsdGVkX1+6hY77unZlRf3E5aQ6pj8NbNdjHafvdGxoG5W7NCsuKSp+0LddF3iva4v6pvwM/xuUsyCp9motpRPLJydI1euPSCseM4FrGoFmxIYrAl7XC4ZAAlB9hUQi+zlM8xGg+GGfd5eti2JQNVH6//y25FsjMv/fVlQZAZAzqO9drxY+eKetgF3LR74S+F9+JXsdtFIiZvTPhLdl+LR0DWfuQfpCQ8e3PJytmwO/OtZwVxmSYmRNuQJMtgI+BW3UKQWuAVh8c7yJHMtqRLR9w3cKDlHyuY2UQ3DC/s6+L+pxeu9GzL2tpAVy1kKs1pdp0yOCw01Ia0h4hcWDxwRYbUEWOFaf3L1QtuWbZsrGNnn5ApqXFkNpNhlr40kasRFF4bl8F75qxsXWqpY7WEGWSz2cjZt9ckWRqO+PpEAtJj4rbzY4PEqZApA6acTV3IJ6Htxk6ct4l/XIjwn7B9fmCoaMj+rVASCbJiOKZ94UlE7nvcM8PkDdcJIbCF6wWGluUT0lOpQTpoDYxuFe2dkSvURcQpCXFMqM2qFy88ACQgtUrttiKInQWMFPnoaKBThcrjQhqB4HPR/veZ2zWjY8TOrYiEMqlf+oJhHpAVx9CPGUvJibJ+qwYb3Witj+M5q1cdaCk4jP8C3mpMfkVDTDHxMniBCcomxKWg9ugdaMAOaXTuJILqYGIUWq1/mKa4oUUttNBPL6MSWiJ302l81c5goS6fDXkYvOkXHMdcdVVS5crR2PiYuKjHMyuvqbYFgtg+AnTpVKOZU08jxnkA0bx+UD7bKHPxai2zFcF3kyo0fHz+H/wNPtqukL4GQUR63SYTAJJWIcGFPao43iy1OpH4/7z8ru5d/SdHdGsZvkj7l9xeD6pKv369iKasb7deN/dsULl7/wanAIEaVoNBCoHI01rZz18FWxinsr2i8VQkex1c6thUgT2WwIAgTGp4FC2k+xbfinbp8YtGz oMpd+Sws jzc9/7yHLq+5vsdMRFDAxNcqWu3TE6D1a4BMFf9O+0Ha/Xl0V/Q46uCwUQYdVaopX2Hu/8q1lJ211hcQX1CycakuIQeCpJJqUjTOsAe+smXUd/XReMcG4er1g/1chia3ANECEM+yeQNNWJe8/scMQtjs+eiHihVHR8S+5eeZKWiyeBZ+aghnWYSFGRZhk6WDpj7M4VhNt/SqplHKOJ78QyhAMWcbchZ+8N0lc63A7FvGN8S4LQLYA9nNv7sxEGQGBfULUz5JF7ZzEt7E8B2v+TmtSACPrxzfJMWdqnkhbm2tq1Z5Pb7W3psw2z7vf/1J3hDKnUmdkswdA10DJeSrvZYzh1qObsumrc/XBECxMxAhwfft4QGXQy0SVQiFrW4w3vnPm+ED5629z+Tk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000010, 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 Thu, Aug 1, 2024 at 6:01=E2=80=AFAM Jann Horn wrote: > > > > @@ -503,15 +509,22 @@ bool __kasan_mempool_poison_object(void *ptr, u= nsigned long ip) > > > kasan_poison(ptr, folio_size(folio), KASAN_PAGE_FREE,= false); > > > return true; > > > } > > > > > > if (is_kfence_address(ptr)) > > > return false; > > > + if (!kasan_arch_is_ready()) > > > + return true; > > > > Hm, I think we had a bug here: the function should return true in both > > cases. This seems reasonable: if KASAN is not checking the object, the > > caller can do whatever they want with it. > > But if the object is a kfence allocation, we maybe do want the caller > to free it quickly so that kfence can catch potential UAF access? So > "return false" in that case seems appropriate. Return false would mean: allocation is buggy, do not use it and do not free it (note that the return value meaning here is inverse compared to the newly added check_slab_allocation()). And this doesn't seem like something we want for KFENCE-managed objects. But regardless of the return value here, the callers tend not to free these allocations to the slab allocator, that's the point of mempools. So KFENCE won't catch a UAF either way. > Or maybe we don't > because that makes the probability of catching an OOB access much > lower if the mempool is going to always return non-kfence allocations > as long as the pool isn't empty? Also I guess whether kfence vetoes > reuse of kfence objects probably shouldn't depend on whether the > kernel is built with KASAN... so I guess it would be more consistent > to either put "return true" there or change the !KASAN stub of this to > check for kfence objects or something like that? Honestly I think the > latter would be most appropriate, though then maybe the hook shouldn't > have "kasan" in its name... Yeah, we could add some custom handling of mempool to KFENCE as well. But that would be a separate effort. > Either way, I agree that the current situation wrt mempools and kfence > is inconsistent, but I think I should probably leave that as-is in my > series for now, and the kfence mempool issue can be addressed > separately afterwards? I also would like to avoid changing kfence > behavior as part of this patch. Sure, sounds good to me. > If you want, I can add a comment above the "if (is_kfence_address())" > that notes the inconsistency? Up to you, I'll likely add a note to the bug tracker to fix this once the patch lands anyway. Thanks!