Re: [PATCH 1/4] module: add syscall to load module from fd

From: Kees Cook
Date: Thu Oct 18 2012 - 11:27:56 EST


On Thu, Oct 18, 2012 at 7:26 AM, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
> On 10/18/2012 01:05 AM, Michael Kerrisk (man-pages) wrote:
>>>
>>>
>>> So perhaps what we *should* have is something that points to the module
>>> to a (buffer, length) in userspace, and the equivalent of the current
>>> init_module() would be open() + mmap() + minit_module() + close()?
>>
>>
>> So, I don't get it. What are the args you propose for of minit_module()?
>>
>
> Nevermind, this is what the current init_module() already takes.
>
> So it sounds like Rusty is objecting to the very notion of tying a module to
> a file descriptor the way the proposed finit_module() system call does -- I

The goal for finit_module is to make sure we're getting what's on the
filesystem, not an arbitrary blob, so we can reason about it for
security policy.

> was confused about the functioning of the *current* init_module() system
> call.
>
> Given that, I have to say I now seriously question the value of
> finit_module(). The kernel can trivially discover if the pointed-to memory
> area is a MAP_SHARED mmap() of a file descriptor and if so which file
> descriptor... why can't we handle this behind the scenes?

This makes me very nervous. I worry that it adds needless complexity
(it'd be many more checks besides "is it MAP_SHARED?", like "does the
memory region show the whole file?" "is the offset zero?" etc). Also
are we sure the memory area would be truly be unmodifiable in the case
where the filesystem is read-only?

-Kees

--
Kees Cook
Chrome OS Security
--
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/