Re: Using TASK_SIZE for kernel threads

[Linus Torvalds]

On Fri, Feb 24, 2017 at 8:15 AM, Martin Schwidefsky
<schwidefsky@xxxxxxxxxx> wrote:
>
> Now I fixed this in the s390 code, the patch is queued and will be
> included in next weeks please-pull. But I am wondering about the use
> of TASK_SIZE in kernel threads. For x86 copy_mount_options works
> because the size calculation will give a negative result for 'data'
> pointing to kernel space. Which is corrected by the size limit:
>
>         if (size > PAGE_SIZE)
>                 size = PAGE_SIZE;
>
> Wouldn't it be cleaner to test "get_fs()==KERNEL_DS" and just use
> size=4096 in this case? The detour via TASK_SIZE does not make much
> sense to me.
>
> To find out how big the problem is, I have added a warning to TASK_SIZE
> to create a console messsage if it is called for a task without an mm.
> The only hit has been copy_mount_options.

So copy_mount_options() is a horrible hack. It doesn't have a size
limit, and it can copy binary data, so our good auto-limiting code in
strncpy_from_user() isn't usable either.

It probably *should* use the same user_addr_max() logic that
strncpy_from_user() uses, but that wouldn't actually have helped s390,
because s390 doesn't use the generic strncpy_from_user(), and doesn't
have that user_addr_max() thing.

So from everything I see, I think this is actually a s390 bug in every
way. Your TASK_SIZE_OF() implementation is simply bogus and broken,
and that's the core problem.

For example, you could have just had

   #define user_addr_max()   (current_thread_info()->addr_limit.seg)

like some other architectures, and it would have been all good.

If somebody is willing to add user_addr_max() to all architectures and
make copy_mount_options() use the same logic as
lib/strncpy_from_user.c, then that would certainly be acceptable to
me. As it is, I think it uses TASK_SIZE in ways that are not pretty,
but are what they are..

                Linus