From patchwork Fri Aug 2 20:31:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 13751971 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 331F9C3DA7F for ; Fri, 2 Aug 2024 20:32:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 999C66B00B0; Fri, 2 Aug 2024 16:32:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8FADE6B00B2; Fri, 2 Aug 2024 16:32:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6FE726B00B3; Fri, 2 Aug 2024 16:32:18 -0400 (EDT) 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 4943F6B00B0 for ; Fri, 2 Aug 2024 16:32:18 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id F3841811F1 for ; Fri, 2 Aug 2024 20:32:17 +0000 (UTC) X-FDA: 82408452714.13.BBF4DC2 Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by imf21.hostedemail.com (Postfix) with ESMTP id E334C1C000B for ; Fri, 2 Aug 2024 20:32:15 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oS3YZvLg; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of jannh@google.com designates 209.85.128.52 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722630688; a=rsa-sha256; cv=none; b=qI3LakrfeEqI8Y0L2hxo2hXeUEzpv9LgGUaw7WStVT13f5Th6AhHBG6Dt35ZOByhuBluz8 FAY4Cm4jJsmYdnRMfsdDNEDC5clnrUsNEKLSWM8Ebp/ig/mZ9Y4rNx2XTZXw9V9+DC2E1P uR3rrmSpsDYV0rxmqGmjbsizIltMB0k= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oS3YZvLg; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of jannh@google.com designates 209.85.128.52 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=1722630688; 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=OtIY/lbfQ818ndlf2SV0BX8ni+uEKDx/O9Hr9lNf93M=; b=kQZeP/1BelfvpXbSW1Ky3Dk3Tpt//ZwUlk4200GFDRoxu6AdNECJ3wJ/CTr8+d2RdfOJh8 Gm5tJBFDGVTq4nhB1R2lgUa64bW3QQbi6Ax2RPTNFEUe9V5hMc6i6epqI8cymZyAYDSY1q k6IrmTPz6blUyG41kYd0PlkKk6n6I/s= Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-42807cb6afdso258385e9.1 for ; Fri, 02 Aug 2024 13:32:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722630734; x=1723235534; darn=kvack.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=OtIY/lbfQ818ndlf2SV0BX8ni+uEKDx/O9Hr9lNf93M=; b=oS3YZvLgoIkrNAxxL4G6IesGel1OcYEbKL03Nc+XEQObvaYvNQUPFlg6WfTptlEmtu pDTDU1Ob4a1livMo/uwjT3AJi6s0NIg4oRooz0q4j02x6Xe2yptl4HpsEHTIrMYUKesO Ob+DJ/oK0LFq6W24KMAyf1azI5BKOqeyxkBBMEEEuNi85gt2u7wkFPtYXL57jEf6SrNM 3IOhBUdcxdCzda0+FXKdzwi0bb0ywv2dHdNelnQNhAmZSm9Q46NtWBQD3KtZsnUiPGsE FkaeII7pvi/ce6A3epsL/+MMU2IVTob+T9TE0pPmvZxBxQxEtkPzvpAnGRgyZYx1wS3c kRZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722630734; x=1723235534; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OtIY/lbfQ818ndlf2SV0BX8ni+uEKDx/O9Hr9lNf93M=; b=KJam1YyTmopLpq5BFm6Im8pBJKIeu5yREJtsBomaC7/fHu3MJjcDxF/qYkuI+c5BH2 Y2epGxG52tKLuCLZTt6pq9pVwiJsdDmkHzOO/zuG1bza8AwxdfS5DLCpobm+OFAvQY1n PUQUeWT7fy9SlBNFks0orelH/x12a4b4Uv/10834RUClG2q+GUflAPQvXJS9naNtbnn7 7rOT1lWCLrzyTMRfUvjvAT6cNg2nRp2B9NloIwnPwDdnKqtccDxl7j/4Dvf796p449tL A25M2NnD5NTgXOt/RoRXyF2OsjVVE50kQYcnpAzVC+LZw1FVqFgtKeyLFRV9uc+XY6Cc 9wsg== X-Forwarded-Encrypted: i=1; AJvYcCWh30AW3wFLaDWiPvzFYhQcZndp/3UgcpxY6pidppb4doaIIpyFBxlGS0mTIk1Us0vHzIuYgybRAD4xsyadTTFuAgk= X-Gm-Message-State: AOJu0Yy52lMQ0Av3xmC4E7xezpfqScx89FUsWJBmBEyGQG/IChXbIgJw AT9wb0gIXbuw2P8QWoCUqLpH+ov/vb69MFiwTYNhUyBV4bGSL509w+2+y4P0Kw== X-Google-Smtp-Source: AGHT+IHgObnJcgoST6ZUYoM69nUyHyMSZdwu3GwWheKkgoMDLPs7gmEI4a2Ly4zw5Ykmyy7wwcJ9iw== X-Received: by 2002:a05:600c:a08c:b0:428:e6eb:1340 with SMTP id 5b1f17b1804b1-428ef3cbbdfmr44035e9.4.1722630733443; Fri, 02 Aug 2024 13:32:13 -0700 (PDT) Received: from localhost ([2a00:79e0:9d:4:9337:bd1:a20d:682d]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-428e6e9d1b4sm43954895e9.39.2024.08.02.13.32.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Aug 2024 13:32:12 -0700 (PDT) From: Jann Horn Date: Fri, 02 Aug 2024 22:31:53 +0200 Subject: [PATCH v6 1/2] kasan: catch invalid free before SLUB reinitializes the object MIME-Version: 1.0 Message-Id: <20240802-kasan-tsbrcu-v6-1-60d86ea78416@google.com> References: <20240802-kasan-tsbrcu-v6-0-60d86ea78416@google.com> In-Reply-To: <20240802-kasan-tsbrcu-v6-0-60d86ea78416@google.com> To: Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: Marco Elver , kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jann Horn X-Mailer: b4 0.15-dev X-Developer-Signature: v=1; a=ed25519-sha256; t=1722630727; l=8985; i=jannh@google.com; s=20240730; h=from:subject:message-id; bh=n3HTPxz1vqklcUBCM2b1ohNM905PLTWckUn1zsgT95o=; b=pBuk9xoZ3On9sdWadP+S/JMQOgnWggeyhLUucDdlYyDK+oaS4P6P0S2gqDMVcBntQIES2k5g0 G7Dzmk5va7PAV+bbKgU4Ounb6xSvRgiyFg+cTwdckFsSVg/xFZitzGz X-Developer-Key: i=jannh@google.com; a=ed25519; pk=AljNtGOzXeF6khBXDJVVvwSEkVDGnnZZYqfWhP1V+C8= X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E334C1C000B X-Stat-Signature: nj8udq4bjtoosdyqteyit6g8qdepc9t4 X-Rspam-User: X-HE-Tag: 1722630735-98930 X-HE-Meta: U2FsdGVkX18QvwrloQuRos9M9Z6b7GSHMBkab45EmVl5n2WM3CyvzA0lnGi1LfI3Y36S7tQLgEl8oGSp1v11Z2NMNMGZ2FPI48r94Im5e02IRFto1YrVFNNFPTvweLVpNVhdi+Vh5y3WMOCAcWKrJpPAiJhSi5VTYZOYqc6rozirlS0ee7NbwFy7+q9Da5HIvIwVQLYvPF48rZT5hoeTvBKscM3fXihv3TqxfqNmBJLNtSH1YUEXCW1vFgODxH50w305gWYGxNmlBNTRaXyZwAOgfU6T2zgsFgDKOgSEDTa58tKyglLQEcId30br+WDdBbo0ghPTGmGc6LAPrxBb1WcmbGyUFqn16W4UGGr1qHhYktdYa6ygyuk1UeV0ulnQtiAtVtITAFTCKtBN5rWpKH73qDibs+e3A4mqpnzKMM53/52Tt+NKzbR53aqdwihe1qLTanMv9T2GNwMN38PTHc6Ggvxu09Ewg0hEHvGTLc8yFaRuXQC1qCD1N/mAMoLEymQVdC9vafjVDaa6ehTqjkIF3LI/KMBbfBIIaY2iEOLj7Ue8b0BkD9rvHHSc0T15sXPaBekNJFx90RArOs5F0/0wJX0wwo3wkJmHUZjUHONOjfHJ9yvmVp3x+TYODdM0GxqO+Gi8H6Bc4frcFNyP7SOs9rhqKO2OIOigumN0cYB9AEPgWFWmU0w/fpmhu3v62Ribhu42Pyx+sRzjq86IPJo3fyYZkONNVS4eOYKst2oKTTHbOqSU4ro0GvXkzSW68kxTOSLRPW16vN20XlB82rjkqB7mPWSlxpheJdDjf4JNivgUSUkm4bIR0sTd0OR2hWh/JjBLxJzQt177o5Mub5/YTpGCFf7N+Sdt3U12jhJStR5w7gRRIaiVYaksLnotNQ/Ixn9k0rfHpjIpmp1AG/OUFG0PXoF2Z8L1x4zLcAlJe4+tYf7opFt1mdkkynHbtp9FFNoh9hXjPss2CbP CchHahNB 24w8e2ATr3d1EJT/rcPj7FdLA4rpr473J9odb99MwbRxXJCwX1/QIVX89X7uahI5wSVbbQNFRkijhk/gft8t3Rs+rimjQEUEOJvYfa/IdtwzFYX5tgwHIcbP2RRcO+dR9bQ0T8XQVyWx1bduusnhK/p/oOhoPSSdap0O9m+K3fLJP32Xaq3d/oblIIsEA0d9sbSYcuIHygOp06+E78Y1zYlOp/pIW6kbr7SjITqyjSdDV/JqTAPD+H1+y4Gik5saU2UfLdtPbL/vkahaB8sJvL1lFw9/4YpM8nnkEbOUNCnGkb3E96idUfFW4VfEe7NZS6xydtcfcwyuobdkHKDFD3zLWlr+wKwHbwYtBN3tJJYbTt41/iPg0e1n6RTiOXBVQhvD7ijr0HDGxe50= 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: 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. Inside KASAN, this: - moves checks from poison_slab_object() into check_slab_allocation() - moves kasan_arch_is_ready() up into callers of poison_slab_object() - removes "ip" argument of poison_slab_object() and __kasan_slab_free() (since those functions no longer do any reporting) Acked-by: Vlastimil Babka #slub Reviewed-by: Andrey Konovalov Signed-off-by: Jann Horn --- include/linux/kasan.h | 54 ++++++++++++++++++++++++++++++++++++++++++--- mm/kasan/common.c | 61 ++++++++++++++++++++++++++++++--------------------- mm/slub.c | 7 ++++++ 3 files changed, 94 insertions(+), 28 deletions(-) diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 70d6a8f6e25d..1570c7191176 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -172,19 +172,61 @@ static __always_inline void * __must_check kasan_init_slab_obj( { if (kasan_enabled()) return __kasan_init_slab_obj(cache, object); return (void *)object; } -bool __kasan_slab_free(struct kmem_cache *s, void *object, - unsigned long ip, bool init); +bool __kasan_slab_pre_free(struct kmem_cache *s, void *object, + unsigned long ip); +/** + * kasan_slab_pre_free - Check whether freeing a slab object is safe. + * @object: Object to be freed. + * + * This function checks whether freeing the given object is safe. It may + * check for double-free and invalid-free bugs and report them. + * + * This function is intended only for use by the slab allocator. + * + * @Return true if freeing the object is unsafe; false otherwise. + */ +static __always_inline bool kasan_slab_pre_free(struct kmem_cache *s, + void *object) +{ + if (kasan_enabled()) + return __kasan_slab_pre_free(s, object, _RET_IP_); + return false; +} + +bool __kasan_slab_free(struct kmem_cache *s, void *object, bool init); +/** + * kasan_slab_free - Poison, initialize, and quarantine a slab object. + * @object: Object to be freed. + * @init: Whether to initialize the object. + * + * This function informs that a slab object has been freed and is not + * supposed to be accessed anymore, except for objects in + * SLAB_TYPESAFE_BY_RCU caches. + * + * For KASAN modes that have integrated memory initialization + * (kasan_has_integrated_init() == true), this function also initializes + * the object's memory. For other modes, the @init argument is ignored. + * + * This function might also take ownership of the object to quarantine it. + * When this happens, KASAN will defer freeing the object to a later + * stage and handle it internally until then. The return value indicates + * whether KASAN took ownership of the object. + * + * This function is intended only for use by the slab allocator. + * + * @Return true if KASAN took ownership of the object; false otherwise. + */ static __always_inline bool kasan_slab_free(struct kmem_cache *s, void *object, bool init) { if (kasan_enabled()) - return __kasan_slab_free(s, object, _RET_IP_, init); + return __kasan_slab_free(s, object, init); return false; } void __kasan_kfree_large(void *ptr, unsigned long ip); static __always_inline void kasan_kfree_large(void *ptr) { @@ -368,12 +410,18 @@ static inline void kasan_poison_new_object(struct kmem_cache *cache, void *object) {} static inline void *kasan_init_slab_obj(struct kmem_cache *cache, const void *object) { return (void *)object; } + +static inline bool kasan_slab_pre_free(struct kmem_cache *s, void *object) +{ + return false; +} + static inline bool kasan_slab_free(struct kmem_cache *s, void *object, bool init) { return false; } static inline void kasan_kfree_large(void *ptr) {} static inline void *kasan_slab_alloc(struct kmem_cache *s, void *object, diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 85e7c6b4575c..f26bbc087b3b 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -205,59 +205,65 @@ void * __must_check __kasan_init_slab_obj(struct kmem_cache *cache, /* Tag is ignored in set_tag() without CONFIG_KASAN_SW/HW_TAGS */ object = set_tag(object, assign_tag(cache, object, true)); return (void *)object; } -static inline bool poison_slab_object(struct kmem_cache *cache, void *object, - unsigned long ip, bool init) +/* Returns true when freeing the object is not safe. */ +static bool check_slab_allocation(struct kmem_cache *cache, void *object, + unsigned long ip) { - void *tagged_object; - - if (!kasan_arch_is_ready()) - return false; + void *tagged_object = object; - tagged_object = object; object = kasan_reset_tag(object); if (unlikely(nearest_obj(cache, virt_to_slab(object), object) != object)) { kasan_report_invalid_free(tagged_object, ip, KASAN_REPORT_INVALID_FREE); return true; } - /* RCU slabs could be legally used after free within the RCU period. */ - if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU)) - return false; - if (!kasan_byte_accessible(tagged_object)) { kasan_report_invalid_free(tagged_object, ip, KASAN_REPORT_DOUBLE_FREE); return true; } + return false; +} + +static inline void poison_slab_object(struct kmem_cache *cache, void *object, + bool init) +{ + void *tagged_object = object; + + object = kasan_reset_tag(object); + + /* RCU slabs could be legally used after free within the RCU period. */ + if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU)) + return; + kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE), KASAN_SLAB_FREE, init); if (kasan_stack_collection_enabled()) kasan_save_free_info(cache, tagged_object); +} - return false; +bool __kasan_slab_pre_free(struct kmem_cache *cache, void *object, + unsigned long ip) +{ + if (!kasan_arch_is_ready() || is_kfence_address(object)) + return false; + return check_slab_allocation(cache, object, ip); } -bool __kasan_slab_free(struct kmem_cache *cache, void *object, - unsigned long ip, bool init) +bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init) { - if (is_kfence_address(object)) + if (!kasan_arch_is_ready() || is_kfence_address(object)) return false; - /* - * If the object is buggy, do not let slab put the object onto the - * freelist. The object will thus never be allocated again and its - * metadata will never get released. - */ - if (poison_slab_object(cache, object, ip, init)) - return true; + poison_slab_object(cache, object, init); /* * If the object is put into quarantine, do not let slab put the object * onto the freelist for now. The object's metadata is kept until the * object gets evicted from quarantine. */ @@ -501,17 +507,22 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip) if (check_page_allocation(ptr, ip)) return false; kasan_poison(ptr, folio_size(folio), KASAN_PAGE_FREE, false); return true; } - if (is_kfence_address(ptr)) - return false; + if (is_kfence_address(ptr) || !kasan_arch_is_ready()) + return true; slab = folio_slab(folio); - return !poison_slab_object(slab->slab_cache, ptr, ip, false); + + if (check_slab_allocation(slab->slab_cache, ptr, ip)) + return false; + + poison_slab_object(slab->slab_cache, ptr, false); + return true; } void __kasan_mempool_unpoison_object(void *ptr, size_t size, unsigned long ip) { struct slab *slab; gfp_t flags = 0; /* Might be executing under a lock. */ diff --git a/mm/slub.c b/mm/slub.c index 3520acaf9afa..0c98b6a2124f 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2223,12 +2223,19 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init) __kcsan_check_access(x, s->object_size, KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT); if (kfence_free(x)) return false; + /* + * Give KASAN a chance to notice an invalid free operation before we + * modify the object. + */ + if (kasan_slab_pre_free(s, x)) + return false; + /* * As memory initialization might be integrated into KASAN, * kasan_slab_free and initialization memset's must be * kept together to avoid discrepancies in behavior. * * The initialization memset's clear the object and the metadata,