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 3041DC3DA64 for ; Thu, 1 Aug 2024 04:01:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6E8266B0092; Thu, 1 Aug 2024 00:01:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 670FD6B0093; Thu, 1 Aug 2024 00:01:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4EACB6B0095; Thu, 1 Aug 2024 00:01:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 2AB706B0092 for ; Thu, 1 Aug 2024 00:01:29 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id A842680879 for ; Thu, 1 Aug 2024 04:01:28 +0000 (UTC) X-FDA: 82402327056.09.752ACCB Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf28.hostedemail.com (Postfix) with ESMTP id BFECBC0029 for ; Thu, 1 Aug 2024 04:01:26 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Pb3sMRwf; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of jannh@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722484823; 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=NT9JA8dx+64cxrNGE7PfVbIZYCFMvXqZQ2Ueee7W7a4=; b=EF5/TE2FJXgZ+BaAzdh0ff7a/HOHKqCOMmV/wof2dMKvXGUl2aCCziya9s7ST2b9Yh2/y2 Brh9k1sKo7lCWcKUDiTInXQGqF9QLrDbfK2mjEVUEi5K9Lw3UUncZIYhqNCO3CLUWXh0+Q 38jo70pDyjUvg4VgNVaVv+q3ns8cm3U= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722484823; a=rsa-sha256; cv=none; b=4+a1B5h7HQlKKt/njOHeWyW967wFfaT0U6etJ1OHTTad+l4GCGv0H5ek6nvBYOQiNlWlsg x1zX10hyr0nwFS/BxV4AYfRYPmkVvCc8dEHFLIVwh7RFq5kPoZ1vH0H6lAEuEmAaCrJ0It Si6ULqa4slNWTPhNU77s7VzxqYb3SXg= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Pb3sMRwf; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of jannh@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=jannh@google.com Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-5a18a5dbb23so27796a12.1 for ; Wed, 31 Jul 2024 21:01:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722484885; x=1723089685; 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=NT9JA8dx+64cxrNGE7PfVbIZYCFMvXqZQ2Ueee7W7a4=; b=Pb3sMRwf7ff5eYsk33f7DvdG1ob9wn3QSWu5ZNRiyTkwcZPC2HMflmXgsJy2VVytjc 5xUU2nGYtNKf5v1Mpo6Q9wiyspFWpfsO+RtmxP0VZS0robHvOqLL3s2lAZQmHllCr2L/ WBfdG6tEz1gJpsTpJcGAvVD2rioYkDabylHRyTn8EWAHuMUuxzIEamwt3dH+7Lanog2v sZVJg2NI/dCys0TRvMiLdGLrFTxqcOekxYwkTM4YWQLsexCxExcmVOFshlEZN3xPAicB ABboV9Uhnq6S9Pw+dhEhmHuPdMbzS8bETW01sj5nZZ5r4CND0NKOWrMDQCcByh60LFFj xn6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722484885; x=1723089685; 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=NT9JA8dx+64cxrNGE7PfVbIZYCFMvXqZQ2Ueee7W7a4=; b=cTEAZJdpuPZo5E6ynXUcSUJ1MU1zS+M9KPuD6PkmZBYfnwwTMVM7tzy4WnTqRRZk51 SEtluY42EZYubn8SdNYTlWcXs9qfxwwKXo02D9vqpChYURgQcF59iE1zB2iK8fyP70dn 9vf7nsIAq+suQRPTAlxq89GgjtEaC9GNVONtE36B2KdEU9bd3v70cvPCA5UNHzFswWdu FMYhaz4aNk/wX5P+Z2KJ6Z6NW2qxlKKLEMWP/+3reC94U5VUbhWkYt6FE+5K3wtnynKT +V8JGtr1NnkOyYOwd6Caal7szHFcPXGz78Qxwgh4k+BNyoHT8Db56eUIpghNzaOeXQPj CRJQ== X-Forwarded-Encrypted: i=1; AJvYcCUlnGVl2ldBN6W+frvaakpg2qeJVTVWbfveO7Oe5pVxdrkwOJ7OgXEmlMZkYqsS+lJu4gtkfIceRxJN4+W1kP3bmx4= X-Gm-Message-State: AOJu0Yzba0GiazWed93r1CYp4wClqE1J1P22eg/ZJPE+Qd6HIgmDUTtP RHaeIxHFzyQehs9PZhE1zvcEtCe+sLaim7Jxj46EYtQlE3OPO/U3lsCbyoW7b7iSmyhqNYAYb3l 80cggHLFrs8dkimYyeCgg0TyHOL7BTtpYf+Jw X-Google-Smtp-Source: AGHT+IHTwjSg9+MIXLuiJLBz0eSMeFs2h8fWwePQsUN7qDGET4IqBmqp79U1QYzfUCOS1JwtDSqCkax9U8EJWq1jF8o= X-Received: by 2002:a05:6402:4301:b0:58b:15e4:d786 with SMTP id 4fb4d7f45d1cf-5b740990c57mr35420a12.5.1722484884423; Wed, 31 Jul 2024 21:01:24 -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: Jann Horn Date: Thu, 1 Aug 2024 06:00:48 +0200 Message-ID: Subject: Re: [PATCH v5 1/2] kasan: catch invalid free before SLUB reinitializes the object To: Andrey Konovalov , Marco Elver Cc: 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: rspam07 X-Rspamd-Queue-Id: BFECBC0029 X-Stat-Signature: kgb15ghs51cdh8ao9a5bpzozomqsbuw9 X-Rspam-User: X-HE-Tag: 1722484886-961502 X-HE-Meta: U2FsdGVkX1/Z9a9n3qquGnhztTfXdRhcvbUF51FaJBeNAvPpz1expEULCpcSquIzyenS1nAyCvPFozuFYWJiA3Dy1RkBwUrZkFmOEYpd/+VMzkKlfaRasdGqBWCwwqXDhRj8MK8TwRFAz3fCsFZMxRgmI79lZHikKbdKlyAojowbUykwxGdDPXH56HUBBfqQGFYRZuc9EoB4+WjgOogbjQy/C6tfJyytK9Xnj3YhM/6p4lLS5YH1XLTdmbM+Ic4wEbo2z0nb0FCb7cTCmrs8BzOhcs4WYVDxZzGJbJ3KY4yXxeeFBZpTO3CLwp3pjL0FINEQUAXqvOwnMfXGkWeC+sMdyyCoJ1Wj+WnMingdrwYv4pTdgQOnf0gAMOaaTyMCSU3GF6YNystyWBszh0t8nGcmCHu8n/Pr7eem+lKVbF86pfP9sKfV1OkeFnFrHcSk4mamQ2xB5FYaTy1sVoDeDcMluF9kUWWzZIGES+j3Fp7QRPhYnmNenIsnbKHmqu7+in6JcNkZJPyMx4gMdP7xblXTAcgeLT2dApan2jEIo5QI5JYk8qPv7hGhQb8ffhNTcNK42XrZYYgAHynTeYqAdCHIojPPdIxASBr/yIEzat5SoGtUjsOkigDMDgdrEBmbdL8nFN07Kgvw3y1QUf7Yw4iE+YWFm+EEuLLHnFyYBbq6ClH15gHhSM24nWW1L62LDG6eNHwrbBw7rJ4+nNDlbKOGfv7tra2UEiuD9P4Z3Mx4vw4ANNCe4JNjRPXsf2mOYmpfUTx7XFgrr2dyvg2oDIwCQ7Cecq8IjQ1zf6qIOq3+velEUxz5kBdJcXQIkCFiEPIV6QyKmlnFWB+znrfe4N1D52fYxmds0dgPPVZGdpt5Jaym3ipLKh7/gKpVMheCI34XTR3VrXI/gU5N5G9QrqeM2VS0UHJIoIYupIvLwl+gkGyp8tzyOTsv/DwYBCjhH828UoqV+xxCdSCQgpY rm2Bq2Hv FrfFtye/X4nBwrO6GxObMflnewii3tUT1hHWYz/rLMAC8LpYrbmMB5Ym3sib1ogRMuJM+ubhLaIRJYL0zXjjebiJ+DTjKdw9cWxeWf7cmiV6Bd6ufGss8jadttYnUIXmB4JTcPO1J4+RUiof87lZ55IMIz9KJWjfb0qDoTCGgOsh/2wYz/Rmo3vppv74m0DaRF3XEhNQT1VnBXRUtaqcCdhNy5xlxabGIpAaFi9ofzrpk8zy+pUEjBVBBrBxv4SY/fmaR5Qdcd711gUYOHPHEsg7BX1ZWGysR3xSN X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: (I'll address the other feedback later) On Thu, Aug 1, 2024 at 2:23=E2=80=AFAM Andrey Konovalov wrote: > > On Tue, Jul 30, 2024 at 1:06=E2=80=AFPM Jann Horn wrot= e: > > > > Currently, when KASAN is combined with init-on-free behavior, the > > initialization happens before KASAN's "invalid free" checks. > > > > More importantly, a subsequent commit will want to RCU-delay the actual > > SLUB freeing of an object, and we'd like KASAN to still validate > > synchronously that freeing the object is permitted. (Otherwise this > > change will make the existing testcase kmem_cache_invalid_free fail.) > > > > So add a new KASAN hook that allows KASAN to pre-validate a > > kmem_cache_free() operation before SLUB actually starts modifying the > > object or its metadata. [...] > > @@ -503,15 +509,22 @@ bool __kasan_mempool_poison_object(void *ptr, uns= igned long ip) > > kasan_poison(ptr, folio_size(folio), KASAN_PAGE_FREE, f= alse); > > 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. 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... 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. If you want, I can add a comment above the "if (is_kfence_address())" that notes the inconsistency?