Re: use-after-free in __perf_install_in_context

[Dmitry Vyukov <dvyukov@google.com>]

On Tue, Dec 8, 2015 at 8:56 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Dec 08, 2015 at 07:35:20PM +0100, Dmitry Vyukov wrote:
>> On Tue, Dec 8, 2015 at 7:05 PM, Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>> > On Tue, Dec 08, 2015 at 06:56:23PM +0100, Dmitry Vyukov wrote:
>> >> On Tue, Dec 8, 2015 at 6:54 PM, Alexei Starovoitov
>> >> <alexei.starovoitov@gmail.com> wrote:
>> >> > On Tue, Dec 08, 2015 at 05:12:04PM +0100, Dmitry Vyukov wrote:
>> >> >> On Tue, Dec 8, 2015 at 4:24 AM, Alexei Starovoitov
>> >> >> <alexei.starovoitov@gmail.com> wrote:
>> >> >> > On Mon, Dec 07, 2015 at 05:09:21PM +0100, Dmitry Vyukov wrote:
>> >> >> >> > So it would be _awesome_ if we could somehow extend this callchain to
>> >> >> >> > include the site that calls call_rcu().
>> >> >> >>
>> >> >> >> We have a patch for KASAN in works that adds so-called stack depot
>> >> >> >> which allows to map a stack trace onto uint32 id. Then we can plumb
>> >> >> >
>> >> >> > I was hacking something similar to categorize stack traces with u32 id.
>> >> >> > How are you planning to limit the number of such stack traces ?
>> >> >> > and what is the interface for user space to get stack trace from an id?
>> >> >>
>> >> >>
>> >> >> We don't limit number of stack traces. Kernel does not seem to use
>> >> >> data-driven recursion extensively, so there is limited number of
>> >> >> stacks. Though, probably we will need to strip non-interrupt part for
>> >> >> interrupt stacks, otherwise that can produce unbounded number of
>> >> >> different stacks.
>> >> >> There is no interface for user-space, it is used only inside of kernel
>> >> >> to save stacks for memory blocks (rcu callbacks, thread pool items in
>> >> >> the future).
>> >> >> The design is based on what we successfully and extensively use in
>> >> >> user-space sanitizers for years. Current code is here:
>> >> >> https://github.com/ramosian-glider/kasan/commit/fb0eefd212366401ed5ad244233ef379a27bfb46
>> >> >
>> >> > why did you pick approach to never free accumulated stacks?
>> >> > That limits usability a lot, since once kasan starts using it only
>> >> > reboot will free the memory. ouch.
>> >> > what worked for user space doesn't work for kernel.
>> >>
>> >>
>> >> Freeing and reusing will slow down and complicate code significantly.
>> >> And it is not yet proved to be necessary.
>> >
>> > It's a joke, right? allocating kernel pages without ability to free?!
>>
>>
>> The plan is to smash kernel much earlier than it will run out of memory.
>>
>>
>> I think this scheme will work pretty well.
>> I've counted 34403 calls to
>> kmalloc/kfree/kmem_cache_alloc/kmem_cache_free in kernel. Multiply
>> this by 2 to account for different stacks leading to the same
>> malloc/free and assuming that we store 16-byte header and 20 4-byte
>> frames, this gives us about 6MB. I can live with that. I can live with
>> 60MB as well.
>
> so you're saying you want to add hundreds lines of code to the kernel only
> to be used by kasan for this very specific and narrow use case
> instead of designing generic 'stacktrace<->id' mechanism?
> That's not how kernel operates. Every kernel developer must think about
> code reuse. We cannot bloat kernel with unique hacks.


We would happily share this code with other subsystems, or even better
reuse an existing solutions. But to the best of my knowledge there is
no such existing solution, and I still know basically nothing about
what you were hacking, why and what are your requirements.
Our requirements are:
 - map stack trace to 4-byte id
 - relatively memory efficient for storing of ~100K traces
 - the only performance-critical operation is mapping of an already
existing stack (in particular no atomic RMW on this path)
Non-requirements:
 - any user-space interface
 - removal of stack traces (they all will be reused in future)
We plan to use in KASAN, KTSAN (already uses it), KMSAN.