On Sat, Feb 03, 2024 at 09:26:49AM +0100, Marco Elver wrote: > On Sat, 3 Feb 2024 at 02:08, <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > This is a note to let you know that I've just added the patch titled > > > > mm, kmsan: fix infinite recursion due to RCU critical section > > > > to the 5.15-stable tree which can be found at: > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > > > The filename of the patch is: > > mm-kmsan-fix-infinite-recursion-due-to-rcu-critical-section.patch > > and it can be found in the queue-5.15 subdirectory. > > > > If you, or anyone else, feels it should not be added to the stable tree, > > please let <stable@xxxxxxxxxxxxxxx> know about it. > > > > > > From f6564fce256a3944aa1bc76cb3c40e792d97c1eb Mon Sep 17 00:00:00 2001 > > From: Marco Elver <elver@xxxxxxxxxx> > > Date: Thu, 18 Jan 2024 11:59:14 +0100 > > Subject: mm, kmsan: fix infinite recursion due to RCU critical section > > > > From: Marco Elver <elver@xxxxxxxxxx> > > > > commit f6564fce256a3944aa1bc76cb3c40e792d97c1eb upstream. > > > > 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@xxxxxxxxxx [1] > > Link: https://lkml.kernel.org/r/20240118110022.2538350-1-elver@xxxxxxxxxx > > Fixes: 5ec8e8ea8b77 ("mm/sparsemem: fix race in accessing memory_section->usage") > > Signed-off-by: Marco Elver <elver@xxxxxxxxxx> > > Reported-by: Alexander Potapenko <glider@xxxxxxxxxx> > > Reported-by: syzbot+93a9e8a3dea8d6085e12@xxxxxxxxxxxxxxxxxxxxxxxxx > > Reviewed-by: Alexander Potapenko <glider@xxxxxxxxxx> > > Tested-by: Alexander Potapenko <glider@xxxxxxxxxx> > > Cc: Charan Teja Kalla <quic_charante@xxxxxxxxxxx> > > Cc: Borislav Petkov (AMD) <bp@xxxxxxxxx> > > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx> > > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > --- > > 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; > > } > > KMSAN didn't exist in 5.15 yet, so the above piece of this patch won't > apply. The below part could optionally be kept, just for the sake of > keeping things consistent. Sorry, this was my fault, it didn't apply and wasn't needed and I dropped it after my scripts picked it up. Thanks for also catching this. greg k-h
