Re: [V9fs-developer] finit_module broken on 9p because kernel_read doesn't work?

[Andy Lutomirski <luto@amacapital.net>]

On Sat, Feb 8, 2014 at 9:15 AM, Richard Yao <ryao@gentoo.org> wrote:
> On 02/08/2014 01:51 AM, Andy Lutomirski wrote:
>> On Fri, Feb 7, 2014 at 11:55 AM, Dominique Martinet
>> <dominique.martinet@cea.fr> wrote:
>>Hi,
>>>
>>> Andy Lutomirski wrote on Fri, Feb 07, 2014:
>>>> I can't get modules to load from 9p.  The problem seems to be that a call like:
>>>>
>>>> kernel_read(f.file, 0, (char *)(info->hdr),, 115551);
>>>>
>>>> is filling the buffer with mostly zeros (or, more likely, just doing
>>>> nothing at all).  The call is in module.c, and the fs is mounted with:
>>>>
>>>> mount -t 9p -o ro,version=9p2000.L,trans=virtio,access=any hostroot /newroot/
>>>>
>>>> This is really easy to test: grab a copy of virtme
>>>> (https://git.kernel.org/cgit/utils/kernel/virtme/virtme.git/), build
>>>> an appropriate kernel, and run it with virtme-runkernel.  Then try to
>>>> insmod any module built for that kernel.  It won't work.
>>>>
>>>> Oddly, running executables from the same fs works, and *copying* a
>>>> module to tmpfs and insmoding it there also works.
>>>>
>>>> I'm kind of at a loss debugging this myself.  I'd expect that if
>>>> kernel_read were that broken on 9p, then I'd see more obvious
>>>> problems.
>>>>
>>>> This problem exists in at least 3.12 and a recent -linus tree.
>>>
>>> That's been reported a couple of times[1] since two months ago, there's a
>>> fix that might or might or might not make it in the tree (Eric?) there:
>>> http://www.spinics.net/lists/linux-virtualization/msg21716.html
>>>
>>> I'm pretty confident that will do it for you, but would be good to hear
>>> you confirm it again :)
>>
>> That fixes it for me.  I think it can't be a module address in
>> finit_module, though -- it's an intermediate vmalloc buffer.  It
>> could, however (in principle) be an address in module data, so the
>> full check is probably good.
>>
>> Can one of you send this to Linus and tag it for -stable?  I can
>> trigger this bug without getting an OOPS, which means that 9p is
>> overwriting random memory, which puts it in the category of rather bad
>> bugs.  I suspect that this is because I don't have
>> CONFIG_DEBUG_VIRTUAL set.
>>
>> (I can't immediately spot any code that would trigger this from user
>> space without being root, so it's probably not a security bug.)
>>
>> --Andy
>>
>
> I have already submitted it for inclusion a couple of times.
>
> The first time was my first time doing any sort of Linux patch
> submission. At the time, I was unaware of ./scripts/get_maintainer.pl
> and sent the patch to only a subset of the correct people. Consequently,
> it was not submitted properly for acceptance by the subsystem maintainer.
>
> The second time was a week ago. I had taken advice from Greg
> Koah-Hartman to use ./scripts/get_maintainer.pl to determine the correct
> recipients. It was initially accepted by the subsystem maintainer and
> then rejected. This patch uses is_vmalloc_or_module_addr(), which is not
> exported for use in kernel modules. Using it causes a build failure when
> CONFIG_NET_9P_VIRTIO=m is set in .config.
>
> I will make a third attempt to mainline this over the next week. Later
> today, I will submit a patch exporting is_vmalloc_or_module_addr().
> After it has been accepted into mainline, I will resubmit this patch,
> which should then be accepted. This should bring this patch into Linus'
> tree sometime in the next few weeks.

I would consider asking some mm people (cc'd) how this is supposed to
work -- that is, what the appropriate way of mapping a kernel virtual
address to a struct page is.

I suspect that the answer might be unpleasant: what happens if the
address is neither in the linear map nor in vmalloc space?  For
example, it could be ioremapped.  (I have no idea under what useful
conditions the 9pnet code wants to zero-copy a buffer, but I suspect
that there are exactly zero performance-critical users of kernel_read
and kernel_write.  Presumably this is for skbs or something.)  I
suspect that the right fix is to just fall back to non-zero-copy if
the page is neither vmalloc'd nor linear-mapped, which should be
doable without new exports.

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>