From patchwork Tue Jul 30 11:06:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 13747259 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 2AB96C3DA7E for ; Tue, 30 Jul 2024 11:06:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 94FF26B0083; Tue, 30 Jul 2024 07:06:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D92B6B0085; Tue, 30 Jul 2024 07:06:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 72C146B0088; Tue, 30 Jul 2024 07:06:38 -0400 (EDT) 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 4EDE96B0083 for ; Tue, 30 Jul 2024 07:06:38 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 688E4C028B for ; Tue, 30 Jul 2024 11:06:37 +0000 (UTC) X-FDA: 82396140834.28.ED070AE Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by imf24.hostedemail.com (Postfix) with ESMTP id 679FB180032 for ; Tue, 30 Jul 2024 11:06:35 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=e4Il3M5V; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.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=1722337555; a=rsa-sha256; cv=none; b=ICxEcH2WBFn45bQ2L2sQMIhFbtuvUFu7UIlfbB4avHYL6DqrsRMOqwqBnVwxWKyHXQ7reX idLUOCx+om+URwjXr0cf4WRnCNRZAVzSe7yw3GbJGRpk9qJMWj26rlY06JosyzhTlex8Qh 5mqzR7MzPAT+TAw2OxI10k6m3WuGWbU= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=e4Il3M5V; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.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=1722337555; 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=bNxWbBeV69N1GBya0KWgugonlwiE99usGrtCl3FAeEo=; b=0QkkYiLm2oBYTjRmp3hqe10dhEZrsBGp7qppEhhjdu+uiB4aw267VuS+HuIA6sEXIQDhrm SqpBjKfzJrGKkt2Uj8QlHfb1H1PwEDUnwxSO9qi/xQF0PUYJCffYd0jVnSLlsPGeBSixgE 9QkfurXinj+cWYEeYNqtRw2T5UKGqcU= Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-428063f4d71so41285e9.1 for ; Tue, 30 Jul 2024 04:06:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722337594; x=1722942394; 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=bNxWbBeV69N1GBya0KWgugonlwiE99usGrtCl3FAeEo=; b=e4Il3M5VM8ke1S+jvVQ/ru5O+3MhcQXOecbjbEsfRfmrL9OZg/GuyO0l3nf3Xl2NeF Hef6YiTN/5kr/uox1zovSFA4Z54QMWdLEIrIzIKF3n2U3xdxodvYPBZ79OCzVs4PphKC ZqyuBA8vqvHY6dLVLOjRKozeNgm31X+PF+iLodG+bbvozaWpjOlKVhVHH/Lk89qb3lKd hbhrMB84FCYo1BJ1jiKAodyIbcmT8NE/tP7/Dhew0vr19O554zhsoGYOl9lrJ8B8oay+ mODIxwckVhbPvJQDFdvp/VhAUYUDDjClcc75UVtvSdMcZCzxOVjYGk6ZiRcAN69G/ta1 0ISQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722337594; x=1722942394; 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=bNxWbBeV69N1GBya0KWgugonlwiE99usGrtCl3FAeEo=; b=OcaJKX9Dd7ym3GBriIe7byCoppise1B8/v2w9k8g9Xps/2GAJ4sHlnTSlHaZr+6opt hCCVrj0aVlhtM1tjxjuutnQCObyTl/N7KxcJS5ZdzL8SAo0iXgsF0upEodBN6AwmelxM T6AET7SgJQk5/14VZ3ibZOd9xIn1zboNDlPdI5o7vbFPQkVVrO2j68ud83+qVZUeew+Z wdXKiAf9qPhXAdc1dvwup08yZi5LvGPnq8b2PsHnG6QnHYiFLETlaiqN/LpUX77mkL4k mKDf+Lbp/T5PseImdZUSYSGalwmXb7MBVicZ8dNvaa2VVc8y+dzetHVVkWSfprlpWY+a ht5A== X-Forwarded-Encrypted: i=1; AJvYcCU6OqLoyo53+6P3wi7NuFHy8Q8Io/Lo7UcCPA0R9zw2LHcXL5XbNvwUGdJ2dnhidgT9t2qykToINCjaLIVLmBzMYSg= X-Gm-Message-State: AOJu0YyAZL7gVwTHGOUVjjsuQdgVUY+Z432CPxZPxIHPEIVIvWqU3sOr V5kPXnVfUjGW/v68H8nJ1ynP28FfVUkklxvm/TBdUlzatvbGRA0J+zap47TP2Q== X-Google-Smtp-Source: AGHT+IGX25ec+jxtgUmtMWmzYxEzG1X0DXLgKUM3362sARyoK5H4+Oz4k0JLP3S/tYUHDT17ECHgew== X-Received: by 2002:a05:600c:1d1a:b0:424:898b:522b with SMTP id 5b1f17b1804b1-42824a359b1mr828395e9.1.1722337593356; Tue, 30 Jul 2024 04:06:33 -0700 (PDT) Received: from localhost ([2a00:79e0:9d:4:be6a:cd70:bdf:6a62]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-428054b9196sm209130635e9.0.2024.07.30.04.06.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jul 2024 04:06:32 -0700 (PDT) From: Jann Horn Date: Tue, 30 Jul 2024 13:06:03 +0200 Subject: [PATCH v5 1/2] kasan: catch invalid free before SLUB reinitializes the object MIME-Version: 1.0 Message-Id: <20240730-kasan-tsbrcu-v5-1-48d3cbdfccc5@google.com> References: <20240730-kasan-tsbrcu-v5-0-48d3cbdfccc5@google.com> In-Reply-To: <20240730-kasan-tsbrcu-v5-0-48d3cbdfccc5@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-Rspamd-Queue-Id: 679FB180032 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: xm9gpehox5dy4tnpas77atx817yka5td X-HE-Tag: 1722337595-343098 X-HE-Meta: U2FsdGVkX1/l6c3TqRXwAfZvJlZUYc/YET4wnnhxktAhTC6vqUAoSPqj5/3fVjpLcG96Bw5nTOHacBCnAsWUqTK58s8TULsadmBX+WwBr86fXTo55bMQ4v1rv21asUJ0mzo6EQko2EE4QHfv2rVk3s/Nh+vfdEMB7Phb5fKPkt4F8WqkhCJksX92eGUFYiri7ZBwgRslRk5PspVXvnw3VSyPxc7yBarnHCfqOvSnVhO9ldjRDU99HUSEI51SgIk7og0dH7yFGVhy2ktSaboQU7dewDV5Xm0OSjjcLzCvTR3wIDsJrbA5LOduN5MeFbHQw/hWslDp1cUO7b+Q2LZKNEAnajQQjDUPCCIJHl7nZD0pt5WcSrQybwtP6eQtjsK+sVXC7I1cgAgD/upIjY3+MsJ/RvHzlW3de4eb8YA/3F3oET98TA1wQs5mi3qF1Ajojh78KIMbEBbrbMzADfDXCCla5LD/x76LHNEZLXTn31XBYNpgfujaUcDrVNirHK2Yx8a0Aj4DL5dlE8/C0IFfXf9Dn8kwyw19ymJhvC93KGn+9ZEm846FP42KSGovBD8+EvptD2svbW4LJCJVvOJv4fNYmASV3LmOOtUJSnPN/+TAGcksvEViVzvN5r+MjgIMx+q8gbShnBS14E7hUz8qU68mkNtXJ5w3EkZU/QNMRuF/hdTQ/RHHTHeioZGMOdyLQQaWUDCanKVTPy/Zj1KDdBZ8sWUUJYEsgTiVVYZBx4ZDV/bLFXF9KAXJvaH+MAGHpgBGnf1iaDpw7XlNKp+nkjyrbghU4w6u0uO3JFc1wqPdocaJdlj1GAz8w7cS4EOexibcI8dwgYIRvVo2DHy8+jr6edxT9+oqUJ6PngtQ7UeqMmwYZHeh5V9A9h/oQyzwgE88BO6yghNbrmW3K0KV54Z3X5RN+mmMQrTXNry5BTrHMuF2dKCxvO7jghiAtpqoRB4Eb/uybzHgUBbbpb7 QOwAwjWr 1LSrZ1ryizKTKEPpmoKk5gfVzQ83rOH4DJTH8EVhnMU7dQo+5N4oz9E0Fu622IKEx4mVUP/0U+rCywC0Zr8Y8XlVG1GfLUcxvW/a5fgrtkG6sUCJ1iOJu2XbAy1RZVhtMz6f9kYXTqeyXdJnb8OKq40xHa+TFWJ+GePUvTLCQ6ARjk/2hkBqKZi3NpltiWVckRSrH6WEwdFmfoptI8xmIC8HlmK2XIlslGpOharBdOVOo1UFmuwUtQBZoTREWQvhI/zkb4anbJQtw2yVn6ih/ly0SfBvKv+1WLTPzLImJ1v2b4YJK0r96QuqOeDstzgXi3clrmW/8sr5uktAaAWmvTxy+YGn7iJsW+WvfKIblRfU1j5Al7r9pfjh1hg== 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_free() - 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) - renames check_slab_free() to check_slab_allocation() Acked-by: Vlastimil Babka #slub Signed-off-by: Jann Horn Reviewed-by: Andrey Konovalov --- include/linux/kasan.h | 43 ++++++++++++++++++++++++++++++++++--- mm/kasan/common.c | 59 +++++++++++++++++++++++++++++++-------------------- mm/slub.c | 7 ++++++ 3 files changed, 83 insertions(+), 26 deletions(-) diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 70d6a8f6e25d..34cb7a25aacb 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -172,19 +172,50 @@ 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 - Validate a slab object freeing request. + * @object: Object to free. + * + * This function checks whether freeing the given object might be permitted; it + * checks things like whether the given object is properly aligned and not + * already freed. + * + * This function is only intended for use by the slab allocator. + * + * @Return true if freeing the object is known to be invalid; 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 - Possibly handle slab object freeing. + * @object: Object to free. + * + * This hook is called from the slab allocator to give KASAN a chance to take + * ownership of the object and handle its freeing. + * kasan_slab_pre_free() must have already been called on the same object. + * + * @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 +399,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..8cede1ce00e1 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 for invalid request */ +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. */ @@ -503,15 +509,22 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned 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; 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,