linux-kernel - Re: [PATCH] kasan: make depot_fetch

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]

Date:	Tue, 5 Jul 2016 10:05:45 +0530
From:	Kuthonuzo Luruo <poll.stdin@...il.com>
To:	Andrey Ryabinin <ryabinin.a.a@...il.com>
Cc:	Dmitry Vyukov <dvyukov@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alexander Potapenko <glider@...gle.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	kasan-dev <kasan-dev@...glegroups.com>
Subject: Re: [PATCH] kasan: make depot_fetch_stack more robust

On Mon, Jul 4, 2016 at 8:11 PM, Andrey Ryabinin <ryabinin.a.a@...il.com> wrote:
> 2016-07-01 20:38 GMT+03:00 Dmitry Vyukov <dvyukov@...gle.com>:
>> I've hit a GPF in depot_fetch_stack when it was given
>> bogus stack handle. I think it was caused by a distant
>> out-of-bounds that hit a different object, as the result
>> we treated uninit garbage as stack handle. Maybe there is
>> something to fix in KASAN logic, but I think it makes
>> sense to make depot_fetch_stack more robust as well.
>>
>> Verify that the provided stack handle looks correct.
>>

> I don't think that adding the kernel code to work around bugs in the
> kernel code makes a lot of sense.
> depot_fetch_stack() fails if invalid handler is passed, and that is a
> bug. You can just add WARN_ON() in
> depot_fetch_stack() if you want to detect such cases..

In this case, the code happens to be a debugging tool that actively anticipates
bad memory accesses. If the tool can reliably detect bad input that could
potentially cause a crash inside the debugger itself, and take actions
to prevent it,
I believe that's a good thing.

> Note that KASAN detects corruption of object's metadata, so such check
> may help only in case of
> corruption page owner's data.

It will also help in case of bad access by non-instrumented code.

>
>>         if (page_ext->last_migrate_reason != -1) {
>>                 ret += snprintf(kbuf + ret, count - ret,
>> @@ -307,12 +308,11 @@ void __dump_page_owner(struct page *page)
>>         }
>>
>>         handle = READ_ONCE(page_ext->handle);
>> -       if (!handle) {
>> +       if (!depot_fetch_stack(handle, &trace)) {
>>                 pr_alert("page_owner info is not active (free page?)\n");
>>                 return;
>>         }
>>
>> -       depot_fetch_stack(handle, &trace);
>>         pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
>>                  page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
>>         print_stack_trace(&trace, 0);
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@...glegroups.com.
> To post to this group, send email to kasan-dev@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/CAPAsAGxj61%3DtrcAAPqODX1Z7vV%3D7-faG1oJBL5WCn%3DrBXAsvNA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.