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 B8980C4707B for ; Thu, 18 Jan 2024 11:08:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 39D676B0071; Thu, 18 Jan 2024 06:08:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 373DC6B0072; Thu, 18 Jan 2024 06:08:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 261A76B007D; Thu, 18 Jan 2024 06:08:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1251F6B0071 for ; Thu, 18 Jan 2024 06:08:15 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id DD17E1A0B65 for ; Thu, 18 Jan 2024 11:08:14 +0000 (UTC) X-FDA: 81692157708.15.471B5E2 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf19.hostedemail.com (Postfix) with ESMTP id 4204A1A0024 for ; Thu, 18 Jan 2024 11:08:13 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UmQzGnrq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of elver@google.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705576093; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=3fBRUIb2Xioj9IoQ1nhfd3l3Jn6qijKU/uajCdRr4Q4=; b=iJNKCpmtpF80tUIZYynXWAQ1AotlT3k4HZzGlyteCh8ASeuxH1rg3t8MfTE9S93SZ7fuRL JoiThocUbrgJQqY+eJsKP1xZbzmyq5yOMa8m5YH7un+m7F6kqBkgxPVUnnW/efWRO79CgY 2Gs3iwvgetbgVpCpDW2wlKx58BCS6AI= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UmQzGnrq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of elver@google.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705576093; a=rsa-sha256; cv=none; b=dUFV7iVY7doQmU5fZb0WAee4c6rUSP7vvv1y+ck0m7gZ+gXbbEosKA6rFm/RH1J42PC8Kz kcCOlgxO34ACMYYrsDbXRNlVnsr/SQP8CTZPVwxkM0naZ0rm5Rr73DhVmFvGVxt9IGn+z3 nb/hCgKkcyNOErmbNnt7H+KzWEjiUV8= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-4b72f2bdbdaso3256565e0c.3 for ; Thu, 18 Jan 2024 03:08:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705576092; x=1706180892; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=3fBRUIb2Xioj9IoQ1nhfd3l3Jn6qijKU/uajCdRr4Q4=; b=UmQzGnrqvwPdT4Z2zKogQkaptl7iRfPGKELKm+OHDHgiBUdDuEy+uwKNgc52OL1TeE SKAxFlCk6iuIQjwc/uG2X+s4RmKO/3qd3jcSoXWyOr/xXjFPjfYOrQdXZn25b+XOAE7E eIIr0vJryHClyrWBLryYUbLTiSCfAE4g9dQigkz1cteCdc4zuisnoHqRjUy9Refhudoc 85sD3MMAsEBvNWCMKR+gFDnqB3wXPTUNNTYLFsD8DR244dgf5hld42NgkWnAj3AWZ8a0 fOXD+wiiykq+2qSJpeZqL/nJzWzQyh6iGYGqeaKr4M6MaYkypZntSpD+ESVAYWd89h6A PPZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705576092; x=1706180892; h=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=3fBRUIb2Xioj9IoQ1nhfd3l3Jn6qijKU/uajCdRr4Q4=; b=frpZm50zYSeDiDdcrHmcI16sv+yK1DNgI7UMQO+1rYJdG66lu3ZYYPsJOlu8u5SVq3 ImiKnB3HFKeBiZDSfBec5SH+cIbsK9MuAFcEPVPENOYaUETb8HC49RpeFNrSyhVfLhVr hZKhvxxsmWl1cjxj48vzYLCwqaYNwnQxW5WysqDDbmG+q0y3Sd6ZC+SzRojwWuqnbhHi ry+jpnMGQ43/JDi1xurkgeS+DtzxMc85cZ2KOHaoAWgfKeav4w87ZvR61R7wGe7NSFr6 iNbjWCc17N04KwrLBN8Nm6rcxwijRTYh2UX9fe2+UAy27d+NNQw0Ox5P1gDd1YegaW0E bumw== X-Gm-Message-State: AOJu0Yx2ZCYx9ubplYPmE1LzB0HoQkp9EEIUTq7bzE0QLy9k0D7AUfRi M2Nl9or0dEyA1VjLG4AOnEiUw5cnSRPJhmo9GDW4k9VQdyGomfr6e7tUIc21ysD4l7XY0LxTiwY qWcz2r4lTbhX5SVMjwFytVh6zl0u+wAX1+uLy X-Google-Smtp-Source: AGHT+IEoAcSXDDVl4x6IPLR2yRPfpy0nUGBJ3WO4L/jouRR6M2ibsaNZ3Hq8iBqXnuEShXsvo4mHyLoW0z/PMHWkbRE= X-Received: by 2002:ac5:cd93:0:b0:4b6:ef57:a068 with SMTP id i19-20020ac5cd93000000b004b6ef57a068mr362816vka.32.1705576092216; Thu, 18 Jan 2024 03:08:12 -0800 (PST) MIME-Version: 1.0 References: <20240118110022.2538350-1-elver@google.com> In-Reply-To: <20240118110022.2538350-1-elver@google.com> From: Marco Elver Date: Thu, 18 Jan 2024 12:07:34 +0100 Message-ID: Subject: Re: [PATCH] mm, kmsan: fix infinite recursion due to RCU critical section To: elver@google.com, Andrew Morton Cc: Alexander Potapenko , Dmitry Vyukov , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, syzbot+93a9e8a3dea8d6085e12@syzkaller.appspotmail.com, Charan Teja Kalla Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 4204A1A0024 X-Stat-Signature: cqj5j3rfyxgxdcgfijyxykiifzzty1ck X-HE-Tag: 1705576093-920661 X-HE-Meta: U2FsdGVkX1822YWApCYqC1FTjyWR4P/K8ijzm8bDriwBS1MJlM6cQ4X8ydIfCW0+3pFuiQLgYY7AVR7BSa3gY5fP/Ndj/3sC82NiMzVGdj7pdE7JmKQpX5xOknRkir4mz33XLmltvtJlCrFtScoS2ljN8soiWoxL8pez8E8iP4ttl+t/8sSung/6sd0GyQIKjHMP/raq2XkZFnvkw+X/6OWhuKlH1CZqANHUVMg73ksTM4lqG8c5GguDRWhlkKhIfUSWMiGBv+Dy1RactVPsGwydyU4s8L+A83ziJgNqcGUnhP6XO1yFrQ8AleHj6yudZoyCXcY1p8COM5RDQVADYh4aEc+14s234zhqr1D6ckT5E7qhMxPZZtd4Mo4HV9CmQoLL4kYJ9RFlBHfOHDdYqGTbqxyRZ7huD83fnojO6bCoYeAL9fQRP/8IlTtIwuSeL0mgPnALYRpV81s6RNFDn3TwnRkf7DPLzhioTNi/E+4Vqp3rW3QoRZaFQx9aUZJuzZdc3vIAnufNHsvUYHHwMTGcqM7ZuHnLLK+2W/W6NKKVO/WOjC3DEK/SwZq2xpcOeoCl/seVnObQVO6utnpWjWuycCaEcucoUvApLD5p8l8+EzH9001+Dw/SaASRIquLMV2nJpZorirpH2SwlXVlSFqBCy4QkQes0zhZQYgVZlqW52GsNhCXSqOq8woneuYt5wAqXYWQF/BL1Inam88y7ZQ4BfLv9K5YpYMSt/8TEGOUvQgwtpGBeKQDHKa+J+xmAnkBZ4iek8PY8mwNsW0lnUJEVa5LBLdeGuzYsUuHlCHvwlcLNstkDFOZ0i54GRC4u5GM04gZB1BjLee5BasJTaq6E0Jkx/9xXu/sLbgXa93HQXeKpG8BhMDul7xQ/KbBFwAnimdU3kFGfS8ME/joL84HYcmrcs9Gt/Obc3c3HlHvjHWerWV9ff8cr8zbfQtiVJQG2eJKsIq0z6lL5SI 0q54k033 r8d2w4gTgD7ftSq5lOYNtK7h4jkD2lk826yEtJBuhFw4m/0migzB32r0pL2e1t6YFuwSXELlfO/zfE8VFUEaj9kU+jvizbAH8hlQGt+ii85kyV9W9ctBaE+qF60k0WVSicHpZuplv1ujJ0+ycRsAq/iMZMs9VW3WTdXT85xH93wFMMIREgIUeNkT8LkfFo5jxxmTOeNDnAR0grsxhIp+fwiSp/4gAFywtOBCUy2rxrm2enmkrTNdxffPUk2/Ae0A288PjVKc9pBH7cnNQYEM42rN9VVKwAWIcAQVN+HB6X9GqNEVYAM4+SyRcFFvcyrDeFMNdXOa+DJzAOb7p8+WVjeHgdtXK1lbvbHAvlVgjtgWY6kkfy7WiQHGGpVpla2EiHNiYlD59vA9yvIhNeJoEBOkaNfqtRCFQvG84Y1VxKv/dOm77xkF5gZNKdZ+MXbiskQgz3yXWe0B49GiTA6o+4o5Pxqa9K1g/zVzVq8FKsAdCeKMv6o7DC08/isLzUsR4lgN1tOdAl+sDLY/Cfwup9xnLog== 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: On Thu, 18 Jan 2024 at 12:00, Marco Elver wrote: > > Alexander Potapenko writes in [1]: "For every memory access in the code > instrumented by KMSAN we call kmsan_get_metadata() to obtain the > metadata for the memory being accessed. For virtual memory the metadata > pointers are stored in the corresponding `struct page`, therefore we > need to call virt_to_page() to get them. > > According to the comment in arch/x86/include/asm/page.h, > virt_to_page(kaddr) returns a valid pointer iff virt_addr_valid(kaddr) > is true, so KMSAN needs to call virt_addr_valid() as well. > > To avoid recursion, kmsan_get_metadata() must not call instrumented > code, therefore ./arch/x86/include/asm/kmsan.h forks parts of > arch/x86/mm/physaddr.c to check whether a virtual address is valid or > not. > > But the introduction of rcu_read_lock() to pfn_valid() added > instrumented RCU API calls to virt_to_page_or_null(), which is called by > kmsan_get_metadata(), so there is an infinite recursion now. I do not > think it is correct to stop that recursion by doing > kmsan_enter_runtime()/kmsan_exit_runtime() in kmsan_get_metadata(): that > would prevent instrumented functions called from within the runtime from > tracking the shadow values, which might introduce false positives." > > Fix the issue by switching pfn_valid() to the _sched() variant of > rcu_read_lock/unlock(), which does not require calling into RCU. Given > the critical section in pfn_valid() is very small, this is a reasonable > trade-off (with preemptible RCU). > > KMSAN further needs to be careful to suppress calls into the scheduler, > which would be another source of recursion. This can be done by wrapping > the call to pfn_valid() into preempt_disable/enable_no_resched(). The > downside is that this sacrifices breaking scheduling guarantees; > however, a kernel compiled with KMSAN has already given up any > performance guarantees due to being heavily instrumented. > > Note, KMSAN code already disables tracing via Makefile, and since > mmzone.h is included, it is not necessary to use the notrace variant, > which is generally preferred in all other cases. > > Link: https://lkml.kernel.org/r/20240115184430.2710652-1-glider@google.com [1] > Reported-by: Alexander Potapenko > Reported-by: syzbot+93a9e8a3dea8d6085e12@syzkaller.appspotmail.com > Signed-off-by: Marco Elver > Cc: Charan Teja Kalla This might want a: Fixes: 5ec8e8ea8b77 ("mm/sparsemem: fix race in accessing memory_section->usage") For reference which patch introduced the problem. > --- > arch/x86/include/asm/kmsan.h | 17 ++++++++++++++++- > include/linux/mmzone.h | 6 +++--- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/kmsan.h b/arch/x86/include/asm/kmsan.h > index 8fa6ac0e2d76..d91b37f5b4bb 100644 > --- a/arch/x86/include/asm/kmsan.h > +++ b/arch/x86/include/asm/kmsan.h > @@ -64,6 +64,7 @@ static inline bool kmsan_virt_addr_valid(void *addr) > { > unsigned long x = (unsigned long)addr; > unsigned long y = x - __START_KERNEL_map; > + bool ret; > > /* use the carry flag to determine if x was < __START_KERNEL_map */ > if (unlikely(x > y)) { > @@ -79,7 +80,21 @@ static inline bool kmsan_virt_addr_valid(void *addr) > return false; > } > > - return pfn_valid(x >> PAGE_SHIFT); > + /* > + * pfn_valid() relies on RCU, and may call into the scheduler on exiting > + * the critical section. However, this would result in recursion with > + * KMSAN. Therefore, disable preemption here, and re-enable preemption > + * below while suppressing reschedules to avoid recursion. > + * > + * Note, this sacrifices occasionally breaking scheduling guarantees. > + * Although, a kernel compiled with KMSAN has already given up on any > + * performance guarantees due to being heavily instrumented. > + */ > + preempt_disable(); > + ret = pfn_valid(x >> PAGE_SHIFT); > + preempt_enable_no_resched(); > + > + return ret; > } > > #endif /* !MODULE */ > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index 4ed33b127821..a497f189d988 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -2013,9 +2013,9 @@ static inline int pfn_valid(unsigned long pfn) > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS) > return 0; > ms = __pfn_to_section(pfn); > - rcu_read_lock(); > + rcu_read_lock_sched(); > if (!valid_section(ms)) { > - rcu_read_unlock(); > + rcu_read_unlock_sched(); > return 0; > } > /* > @@ -2023,7 +2023,7 @@ static inline int pfn_valid(unsigned long pfn) > * the entire section-sized span. > */ > ret = early_section(ms) || pfn_section_valid(ms, pfn); > - rcu_read_unlock(); > + rcu_read_unlock_sched(); > > return ret; > } > -- > 2.43.0.381.gb435a96ce8-goog >