Re: [PATCH] kasan, module, vmalloc: rework shadow allocation for modules

From: Rusty Russell
Date: Thu Feb 19 2015 - 22:47:54 EST


Andrey Ryabinin <a.ryabinin@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/