Re: [PATCH] NoMoreModuleVmalloc - Try alloc_pages_exact before vmalloc, if fails do vmalloc

[Lucas Tanure]

Hi Russell,

I found that project
http://kernelnewbies.org/KernelProjects/NoMoreModuleVmalloc.
So I thought that doing first a alloc_pages_exact would be the goal.
The kernel/module.c doesn't need this task any more, or I just did in
the wrong way ?

Thanks
--
Lucas Tanure
+55 (19) 988176559


On Mon, Jul 14, 2014 at 7:18 AM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
> Lucas Tanure <tanure@xxxxxxxxx> writes:
>> Convert vmalloc() call to alloc_pages_exact(). If alloc_pages_exact() fails,
>> then fall back to vmalloc(). If the address is a vmalloc address, then
>> call vfree(), otherwise call free_pages_exact().
>
> Hi Lucas,
>
>         This patch is wrong.
>
> Firstly, you didn't list a motivation for getting rid of vmalloc.  But
> this patch doesn't make modules avoid vmalloc, it just makes the
> temporary copy we create avoid vmalloc.
>
> It also doesn't avoid the vfree() on the normal path; I'm pretty sure
> this would crash if you tested it.
>
> Thanks,
> Rusty.
>
>> Signed-off-by: Lucas Tanure <tanure@xxxxxxxxx>
>> ---
>>  kernel/module.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index ae79ce6..50c1e77 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2484,6 +2484,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>>                                 struct load_info *info)
>>  {
>>       int err;
>> +     bool alloc_from_vmalloc = false;
>>
>>       info->len = len;
>>       if (info->len < sizeof(*(info->hdr)))
>> @@ -2494,12 +2495,19 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
>>               return err;
>>
>>       /* Suck in entire file: we'll want most of it. */
>> -     info->hdr = vmalloc(info->len);
>> -     if (!info->hdr)
>> -             return -ENOMEM;
>> +     info->hdr = alloc_pages_exact(info->len, GFP_KERNEL);
>> +     if (!info->hdr) {
>> +             info->hdr = vmalloc(info->len);
>> +             if (!info->hdr)
>> +                     return -ENOMEM;
>> +             alloc_from_vmalloc = true;
>> +     }
>>
>>       if (copy_from_user(info->hdr, umod, info->len) != 0) {
>> -             vfree(info->hdr);
>> +             if(alloc_from_vmalloc)
>> +                     vfree(info->hdr);
>> +             else
>> +                     free_pages_exact(info->hdr,info->len);
>>               return -EFAULT;
>>       }
>>
>> --
>> 2.0.1
--
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/