linux-kernel - Re: [PATCH] kasan: Don't call find_vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ef785e05-f52e-4a88-9377-b51b81b228ce@gmail.com>
Date: Wed, 12 Feb 2025 18:52:39 +0100
From: Andrey Ryabinin <ryabinin.a.a@...il.com>
To: Waiman Long <llong@...hat.com>
Cc: Alexander Potapenko <glider@...gle.com>,
 Andrey Konovalov <andreyknvl@...il.com>, Dmitry Vyukov <dvyukov@...gle.com>,
 Vincenzo Frascino <vincenzo.frascino@....com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
 kasan-dev@...glegroups.com, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
 Nico Pache <npache@...hat.com>
Subject: Re: [PATCH] kasan: Don't call find_vm_area() in RT kernel



On 2/12/25 2:34 PM, Waiman Long wrote:
> 
> On 2/12/25 6:59 AM, Andrey Ryabinin wrote:
>> On Tue, Feb 11, 2025 at 5:08 PM Waiman Long <longman@...hat.com> wrote:
>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>> index 3fe77a360f1c..e1ee687966aa 100644
>>> --- a/mm/kasan/report.c
>>> +++ b/mm/kasan/report.c
>>> @@ -398,9 +398,20 @@ static void print_address_description(void *addr, u8 tag,
>>>                  pr_err("\n");
>>>          }
>>>
>>> -       if (is_vmalloc_addr(addr)) {
>>> -               struct vm_struct *va = find_vm_area(addr);
>>> +       if (!is_vmalloc_addr(addr))
>>> +               goto print_page;
>>>
>>> +       /*
>>> +        * RT kernel cannot call find_vm_area() in atomic context.
>>> +        * For !RT kernel, prevent spinlock_t inside raw_spinlock_t warning
>>> +        * by raising wait-type to WAIT_SLEEP.
>>> +        */
>>> +       if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
>>> +               static DEFINE_WAIT_OVERRIDE_MAP(vmalloc_map, LD_WAIT_SLEEP);
>>> +               struct vm_struct *va;
>>> +
>>> +               lock_map_acquire_try(&vmalloc_map);
>>> +               va = find_vm_area(addr);
>> Can we hide all this logic behind some function like
>> kasan_find_vm_area() which would return NULL for -rt?
> Sure. We can certainly do that.
>>
>>>                  if (va) {
>>>                          pr_err("The buggy address belongs to the virtual mapping at\n"
>>>                                 " [%px, %px) created by:\n"
>>> @@ -410,8 +421,13 @@ static void print_address_description(void *addr, u8 tag,
>>>
>>>                          page = vmalloc_to_page(addr);
>> Or does vmalloc_to_page() secretly take  some lock somewhere so we
>> need to guard it with this 'vmalloc_map' too?
>> So my suggestion above wouldn't be enough, if that's the case.
> 
> AFAICS, vmalloc_to_page() doesn't seem to take any lock.  Even if it takes another spinlock, it will still be under the vmalloc_map protection until lock_map_release() is called.
> 

I meant to do something like bellow, which would leave vmalloc_to_page() out of vmalloc_map scope.
That's why I raised this question.

---
 mm/kasan/report.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 3fe77a360f1c..f3683215f4ca 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -370,6 +370,20 @@ static inline bool init_task_stack_addr(const void *addr)
 			sizeof(init_thread_union.stack));
 }
 
+static inline struct vm_struct *kasan_find_vm_area(void *addr)
+{
+	static DEFINE_WAIT_OVERRIDE_MAP(vmalloc_map, LD_WAIT_SLEEP);
+	struct vm_struct *va;
+
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		return NULL;
+
+	lock_map_acquire_try(&vmalloc_map);
+	va = find_vm_area(addr);
+	lock_map_release(&vmalloc_map);
+	return va;
+}
+
 static void print_address_description(void *addr, u8 tag,
 				      struct kasan_report_info *info)
 {
@@ -399,8 +413,7 @@ static void print_address_description(void *addr, u8 tag,
 	}
 
 	if (is_vmalloc_addr(addr)) {
-		struct vm_struct *va = find_vm_area(addr);
-
+		struct vm_area *va = kasan_find_vm_area(addr);
 		if (va) {
 			pr_err("The buggy address belongs to the virtual mapping at\n"
 			       " [%px, %px) created by:\n"
-- 
2.45.3