From: Rusty Russell <rusty@rustcorp.com.au>
To: Andrey Ryabinin <a.ryabinin@samsung.com>,
        Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH] kasan, module, vmalloc: rework shadow allocation for modules
In-Reply-To: <54E5E355.9020404@samsung.com>
References: <1424281467-2593-1-git-send-email-a.ryabinin@samsung.com> <87pp96stmz.fsf@rustcorp.com.au> <54E5E355.9020404@samsung.com>
User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu)
Date: Fri, 20 Feb 2015 10:45:23 +1030
Message-ID: <87fva1sajo.fsf@rustcorp.com.au>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1653
Lines: 42

Andrey Ryabinin <a.ryabinin@samsung.com> writes:
> On 02/19/2015 02:10 AM, Rusty Russell wrote:
>> This is not portable.  Other archs don't use vmalloc, or don't use
>> (or define) MODULES_VADDR.  If you really want to hook here, you'd
>> need a new flag (or maybe use PAGE_KERNEL_EXEC after an audit).
>> 
>
> Well, instead of explicit (addr >= MODULES_VADDR && addr < MODULES_END)
> I could hide this into arch-specific function: 'kasan_need_to_allocate_shadow(const void *addr)'
> or make make all those functions weak and allow arch code to redefine them.

That adds another layer of indirection.  And how would the caller of
plain vmalloc() even know what to return?

>> Thus I think modifying the callers is the better choice.
>> 
>
> I could suggest following (though, I still prefer 'modifying vmalloc' approach):
>   * In do_init_module(), instead of call_rcu(&freeinit->rcu, do_free_init);
>     use synchronyze_rcu() + module_memfree(). Of course this will be
>   under CONFIG_KASAN.

But it would be slow, and a disparate code path, which is usually a bad
idea.

>     As you said there other module_memfree() users, so what if they will decide
>     to free memory in atomic context?

Hmm, how about a hybrid:

1) Add kasan_module_alloc(p, size) after module alloc as your original.
2) Hook into vfree(), and ignore it if you can't find the map.

Or is the latter too expensive?

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/