[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHPzcFkCSG03+HkW=owM7WqOJ1J9-Zo7wL3raQTxEmce+uAfpg@mail.gmail.com>
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.
Powered by blists - more mailing lists